Conversation
- add unknown json field preservation via serde flatten (#92) - add core config models: permissions, attribution, hook definition, hook group (#93) - add claude settings model with hooks lookup and tool checking (#94) - add mcp server and config models with stdio/http support (#95) - add project entry and legacy config models for ~/.claude.json (#96) - add config merge types: config source, merged value, merged settings (#103) - add error types with thiserror: fig error, config file error, bundle error, health check error (#104) - add config file manager with json i/o, backups, and symlink resolution (#105) - add file watcher service using notify crate (#106) - add settings merge service with 3-tier precedence (#107)
- Add DiscoveredProject model with config file detection (#97) - Add ProjectGroup model with group_by_directory grouping (#97, #110) - Add NavigationSelection, EditingTarget, tab enums (#108) - Add ProjectDiscoveryService with legacy config and directory scanning (#98) - Add Iced app shell with sidebar + detail pane layout (#109) - Add sidebar project grouping by parent directory (#110) - Apply cargo fmt to existing phase 1 files
- Add EditablePermissionRule, EditableEnvironmentVariable with UUID identity (#111) - Add PermissionPreset catalog with 6 presets and KnownEnvironmentVariable list (#111) - Add PermissionType and ToolType enums (#111) - Add permissions editor UI with presets, add/remove rules, type toggle (#112) - Add environment variable editor with known var suggestions and sensitive masking (#113) - Add attribution editor with commit/PR toggles and target selector (#114) - Add UndoManager<T> with undo/redo stacks, dirty tracking, max history (#115) - Wire editors into detail view replacing placeholder content
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Rust workspace with a fig-core library for config/models/services and a fig-ui desktop UI (Iced) implementing navigation plus editors for permissions, environment variables, attribution, and MCP server management.
Changes:
- Added
fig-coremodels and services for reading/writing config, merging settings, project discovery, undo/redo, MCP import/export/copy utilities, and file watching. - Added an Iced-based
fig-uiapp with sidebar navigation and multiple editor views (permissions, environment, attribution, MCP servers). - Created a Rust workspace and initial crate manifests/styles for the new Rust implementation.
Reviewed changes
Copilot reviewed 41 out of 42 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| fig-ui/src/views/sidebar.rs | Sidebar navigation + project list (flat/grouped) rendering and path abbreviation. |
| fig-ui/src/views/permissions_editor.rs | Permissions editor state + view (rules list, presets, input). |
| fig-ui/src/views/mod.rs | Exposes view modules. |
| fig-ui/src/views/mcp_server_list.rs | MCP server list UI, expand/collapse cards, edit/delete actions, redacted display. |
| fig-ui/src/views/mcp_server_form.rs | MCP server add/edit form UI with validation error display. |
| fig-ui/src/views/mcp_copy_sheet.rs | Copy/import sheets UI for MCP servers, conflict display, redaction toggle UI. |
| fig-ui/src/views/environment_editor.rs | Environment variable editor state + view, known-var quick add. |
| fig-ui/src/views/detail.rs | Main detail pane routing + global/project tab bars and placeholders. |
| fig-ui/src/views/attribution_editor.rs | Attribution editor state + view for commits/PR attribution toggles. |
| fig-ui/src/styles/mod.rs | Centralized UI constants (colors, widths). |
| fig-ui/src/main.rs | Iced application wiring, message enum, update logic, view composition. |
| fig-ui/Cargo.toml | UI crate manifest + dependencies. |
| fig-core/src/services/undo_manager.rs | Generic undo/redo manager with dirty tracking and tests. |
| fig-core/src/services/settings_merge.rs | Merges global/shared/local settings into a single merged view with source tracking + tests. |
| fig-core/src/services/project_discovery.rs | Project discovery via legacy config and directory scanning + tests. |
| fig-core/src/services/mod.rs | Service module exports/re-exports. |
| fig-core/src/services/mcp_copy_service.rs | MCP server copy/conflict detection helpers + tests. |
| fig-core/src/services/mcp_clipboard_service.rs | MCP server JSON import/export with optional redaction + tests. |
| fig-core/src/services/file_watcher.rs | Notify-based file watcher abstraction + tests. |
| fig-core/src/services/config_file_manager.rs | JSON config read/write, backup creation, symlink resolution + tests. |
| fig-core/src/models/project_group.rs | Groups discovered projects by directory with home-relative display naming + tests. |
| fig-core/src/models/project_entry.rs | Legacy project entry model with serde + helpers + tests. |
| fig-core/src/models/permissions.rs | Permissions model with serde + unknown-field preservation + tests. |
| fig-core/src/models/navigation.rs | Navigation selection and tab enums + editing target helpers + tests. |
| fig-core/src/models/mod.rs | Model module exports/re-exports. |
| fig-core/src/models/merged_settings.rs | Merged settings data types and helper methods + tests. |
| fig-core/src/models/mcp_server.rs | MCP server model (stdio/http) with helpers + tests. |
| fig-core/src/models/mcp_form_data.rs | UI-facing MCP form data conversion + validation + tests. |
| fig-core/src/models/mcp_config.rs | MCP config root model (mcpServers) + helpers + tests. |
| fig-core/src/models/legacy_config.rs | Legacy global config model + helpers + tests. |
| fig-core/src/models/hook_group.rs | Hooks group model with serde + tests. |
| fig-core/src/models/hook_definition.rs | Hook definition model with serde + tests. |
| fig-core/src/models/editable_types.rs | Editable UI types, presets, known env vars + tests. |
| fig-core/src/models/discovered_project.rs | Discovered project model + tests. |
| fig-core/src/models/config_source.rs | Config source enum with precedence/labels + tests. |
| fig-core/src/models/claude_settings.rs | Claude settings root model + helper methods + tests. |
| fig-core/src/models/attribution.rs | Attribution model with serde + tests. |
| fig-core/src/lib.rs | Library module entrypoint. |
| fig-core/src/error.rs | Error types for config/bundles/health checks + recovery suggestions + tests. |
| fig-core/Cargo.toml | Core crate manifest + dependencies. |
| Cargo.toml | Workspace definition. |
| .gitignore | Adds Rust-related ignores. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Message::SelectProject(path) => { | ||
| self.selection = NavigationSelection::Project(path); | ||
| self.project_tab = ProjectDetailTab::Permissions; | ||
| } |
There was a problem hiding this comment.
When selecting a project, the editor states (permissions_state, environment_state, attribution_state) keep editing_target == Global. Because the editor views only offer [Global] when the target is Global, project detail pages won’t let users switch to ProjectShared/ProjectLocal targets. Consider setting these editing_targets to a project-appropriate default (and resetting back to Global when returning to global settings).
| Message::MCPExportToClipboard => { | ||
| let servers: Vec<(&str, &fig_core::models::MCPServer)> = self | ||
| .mcp_list_state | ||
| .servers | ||
| .iter() | ||
| .map(|(n, s)| (n.as_str(), s)) | ||
| .collect(); | ||
| let _json = mcp_clipboard_service::export_to_json(&servers, false); | ||
| // In a full implementation, this would copy to system clipboard | ||
| } |
There was a problem hiding this comment.
MCPExportToClipboard always calls export_to_json(..., false), so it will export secrets even if the UI has a redaction toggle (currently stored in ImportSheetState.redact_on_export). Either wire the toggle into export (and store it in a state that’s available here) or default to redaction for clipboard exports to reduce accidental secret exposure.
| fn grouped_project_list<'a>( | ||
| projects: &'a [DiscoveredProject], | ||
| selection: &'a NavigationSelection, | ||
| ) -> Element<'a, Message> { | ||
| let groups = group_projects_internal(projects); | ||
| let mut col = Column::new(); | ||
|
|
||
| for group in &groups { | ||
| col = col.push( | ||
| container( | ||
| text(group.display_name.clone()) | ||
| .size(11) | ||
| .color(styles::GROUP_HEADER_TEXT), | ||
| ) | ||
| .padding(Padding::new(8.0).left(20.0).right(20.0).bottom(2.0)), | ||
| ); | ||
|
|
||
| for dp in &group.discovered { | ||
| let path_str = dp.path.to_string_lossy().to_string(); | ||
| let is_selected = selection == &NavigationSelection::Project(path_str.clone()); | ||
| col = col.push(sidebar_item( | ||
| &dp.display_name, | ||
| Some(&dp.path), | ||
| is_selected, | ||
| Message::SelectProject(path_str), | ||
| )); | ||
| } | ||
| } | ||
|
|
||
| col.into() | ||
| } | ||
|
|
||
| /// Internal grouping struct for sidebar rendering that keeps DiscoveredProject refs. | ||
| struct SidebarProjectGroup<'a> { | ||
| display_name: String, | ||
| discovered: Vec<&'a DiscoveredProject>, | ||
| } | ||
|
|
||
| fn group_projects_internal(projects: &[DiscoveredProject]) -> Vec<SidebarProjectGroup<'_>> { | ||
| let home = dirs::home_dir(); | ||
| let mut groups_map: BTreeMap<PathBuf, Vec<&DiscoveredProject>> = BTreeMap::new(); | ||
|
|
||
| for project in projects { | ||
| let parent = project | ||
| .path | ||
| .parent() | ||
| .unwrap_or(Path::new("/")) | ||
| .to_path_buf(); | ||
| groups_map.entry(parent).or_default().push(project); | ||
| } | ||
|
|
||
| groups_map | ||
| .into_iter() | ||
| .map(|(parent_path, members)| { | ||
| let display_name = abbreviate_dir(&parent_path, home.as_deref()); | ||
| SidebarProjectGroup { | ||
| display_name, | ||
| discovered: members, | ||
| } | ||
| }) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
This sidebar re-implements project grouping and home-directory abbreviation (group_projects_internal / abbreviate_dir), which duplicates logic already in fig-core (ProjectGroup::group_by_directory + its abbreviate_dir). To avoid drift and keep grouping behavior consistent across the app, consider reusing the core model/service here instead of maintaining a parallel implementation.
fig-ui/src/views/mcp_server_list.rs
Outdated
| fn is_sensitive_key(key: &str) -> bool { | ||
| let upper = key.to_uppercase(); | ||
| ["TOKEN", "KEY", "SECRET", "PASSWORD"] | ||
| .iter() | ||
| .any(|pat| upper.contains(pat)) | ||
| } |
There was a problem hiding this comment.
is_sensitive_key duplicates the same sensitive-key detection logic already available as EditableEnvironmentVariable::is_sensitive_key in fig-core. Reusing the shared helper would reduce the chance of the UI and core logic diverging over time.
| pub fn unwatch_all(&mut self) { | ||
| let paths: Vec<PathBuf> = self.watched_paths.drain().collect(); | ||
| for path in paths { | ||
| let watch_path = if path.is_file() { | ||
| path.parent().unwrap_or(&path).to_path_buf() | ||
| } else { | ||
| path | ||
| }; | ||
| let _ = self.watcher.unwatch(&watch_path); | ||
| } |
There was a problem hiding this comment.
Same path.is_file() issue in unwatch_all(): if watched file paths no longer exist, the code will compute the wrong watch_path and fail to unwatch the directory that was originally watched. This can leak active watches until process exit. Prefer unwatching the exact watch_path recorded at watch() time.
| let content = fs::read_to_string(path).map_err(|e| { | ||
| if e.kind() == std::io::ErrorKind::PermissionDenied { | ||
| ConfigFileError::PermissionDenied { | ||
| path: path.to_path_buf(), | ||
| } | ||
| } else { | ||
| ConfigFileError::InvalidJson { | ||
| path: path.to_path_buf(), | ||
| message: e.to_string(), | ||
| } | ||
| } | ||
| })?; |
There was a problem hiding this comment.
read() maps most fs::read_to_string failures (anything except PermissionDenied) into ConfigFileError::InvalidJson, which is misleading for plain IO errors (e.g., transient read failure, broken symlink, etc.). Consider adding a dedicated read error variant (or reusing WriteError with a clearer name) so the error and its message accurately reflect the failure mode.
| if let Some(preset) = PERMISSION_PRESETS.iter().find(|p| p.id == preset_id) { | ||
| for (rule, ptype) in preset.rules { | ||
| if !self.rules.iter().any(|r| r.rule == *rule) { | ||
| self.rules | ||
| .push(EditablePermissionRule::new(rule.to_string(), *ptype)); | ||
| } | ||
| } |
There was a problem hiding this comment.
apply_preset iterates preset.rules as if it yields tuples, but it actually yields references to tuples (&(&str, PermissionType)), so for (rule, ptype) in preset.rules will not compile. Destructure the reference (e.g., for &(rule, ptype) in preset.rules) or iterate with .iter().copied() and adjust the body accordingly.
| pub fn watch(&mut self, path: &Path) -> Result<(), notify::Error> { | ||
| let watch_path = if path.is_file() { | ||
| path.parent().unwrap_or(path) | ||
| } else { | ||
| path | ||
| }; | ||
|
|
||
| self.watcher | ||
| .watch(watch_path, RecursiveMode::NonRecursive)?; | ||
| self.watched_paths.insert(path.to_path_buf()); | ||
| Ok(()) |
There was a problem hiding this comment.
FileWatcher::unwatch decides whether to unwatch path vs path.parent() using path.is_file(). If a watched file is deleted (or didn’t exist at unwatch time), is_file() becomes false and the code will try to unwatch the file path instead of the directory that was actually watched, leaving the watch active. Consider storing the effective watch_path used in watch() (or storing a mapping) and unwatching that exact path later.
| [dependencies] | ||
| fig-core = { path = "../fig-core" } | ||
| iced = { version = "0.13", features = ["tokio", "svg"] } | ||
| iced_aw = "0.11" | ||
| dirs = "6" | ||
| uuid = { version = "1", features = ["v4"] } |
There was a problem hiding this comment.
iced_aw is declared as a dependency but doesn’t appear to be used anywhere in fig-ui (no references in the crate). If it’s not needed yet, consider removing it to keep the dependency set minimal; otherwise, add the usage in this PR to justify the new dependency.
- Reuse shared abbreviate_dir and is_sensitive_key instead of duplicating - Preserve headers and additional_properties on MCP form round-trip - Store effective watch paths in HashMap for correct unwatch behavior - Add ReadError variant for non-permission file read failures - Fix preset iteration destructuring in permissions and editable types - Set editing_target on navigation to prevent stale state - Default MCP export to redact sensitive values - Remove unused iced_aw dependency
- replace .unwrap() with proper error handling in mcp_health_check.rs (CLAUDE.md: no .unwrap() in library code) - fix MCPExportToClipboard to actually write to system clipboard via arboard instead of discarding the result - fix HealthCheckRun to use actual app state instead of empty defaults - fix MCPCopyConfirm/MCPCopyForceOverwrite to perform conflict detection and server copying instead of just closing the sheet - fix resolve_symlink to handle relative symlink targets by resolving against the symlink's parent directory - use atomic write (temp file + rename) in ConfigFileManager to prevent data loss on interrupted writes - add tests for relative symlink resolution, atomic writes, and health check error handling (162 -> 166 tests) - update CLAUDE.md test count
No description provided.