Skip to content

Commit c63e0c6

Browse files
committed
feat(config): implement Phase 5 — config system complete
Implement all 23 todo!() items across 6 files: - keybinding.rs: keystroke parser (ctrl+shift+a format), chord parser (space-separated sequences), KeybindingResolver with defaults, user override loading from JSON, last-wins conflict resolution, context fallback to Global, validate_config_entry - validation.rs: full settings schema validation (known fields, type checks, maxTokens range, permissionMode/apiProvider enums, MCP server config, hook entry validation), permission rule syntax check - settings_cache.rs: lazy load with Mutex, merge global+project, invalidate for hot-reload - feature_flag.rs: load_from_settings parsing featureFlags object, merge with override semantics - change_detector.rs: file fingerprinting via DefaultHasher, per-source comparison, mark_known suppression - mdm.rs: platform-specific paths (macOS plist, Windows ProgramData, Linux /etc), first-available loading, drop-in directory merge, is_managed_environment detection policy.rs (564 LOC) was already complete. 60+ new tests added.
1 parent e7c22c2 commit c63e0c6

6 files changed

Lines changed: 1073 additions & 208 deletions

File tree

crates/config/src/change_detector.rs

Lines changed: 94 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88
//! and compared against the last-known state.
99
1010
use std::collections::HashMap;
11+
use std::collections::hash_map::DefaultHasher;
12+
use std::hash::{Hash, Hasher};
13+
use std::path::Path;
1114

1215
// ── Change event ──────────────────────────────────────────────────────
1316

@@ -24,19 +27,6 @@ pub struct SettingsChange {
2427

2528
/// Detects changes in individual settings layers by comparing content
2629
/// fingerprints between successive checks.
27-
///
28-
/// # Usage
29-
///
30-
/// ```rust,no_run
31-
/// use crab_config::change_detector::ChangeDetector;
32-
///
33-
/// let mut detector = ChangeDetector::new();
34-
/// let changes = detector.check_for_changes();
35-
/// for change in &changes {
36-
/// println!("source {} changed keys: {:?}", change.source, change.changed_keys);
37-
/// }
38-
/// ```
39-
#[allow(dead_code)]
4030
pub struct ChangeDetector {
4131
/// Last-known fingerprint per source name.
4232
known_hashes: HashMap<String, u64>,
@@ -53,17 +43,62 @@ impl ChangeDetector {
5343

5444
/// Check all settings sources and return a list of changes since the
5545
/// last call. Sources that have not changed are omitted.
46+
///
47+
/// Reads known config paths and computes a hash fingerprint for each.
48+
/// On first call, all readable sources are reported as changed (to
49+
/// trigger initial loading).
5650
pub fn check_for_changes(&mut self) -> Vec<SettingsChange> {
57-
todo!("check_for_changes: fingerprint each source and compare with known_hashes")
51+
let mut changes = Vec::new();
52+
53+
// Check each known source path
54+
let sources = [
55+
(
56+
"global",
57+
crate::settings::global_config_dir().join("settings.json"),
58+
),
59+
// Project source is checked if we have a project dir
60+
// (callers can use check_source() for project-specific paths)
61+
];
62+
63+
for (name, path) in &sources {
64+
if let Some(change) = self.check_source(name, path) {
65+
changes.push(change);
66+
}
67+
}
68+
69+
changes
70+
}
71+
72+
/// Check a single source file for changes.
73+
///
74+
/// Returns `Some(SettingsChange)` if the file's fingerprint differs
75+
/// from the last known state.
76+
pub fn check_source(&mut self, source: &str, path: &Path) -> Option<SettingsChange> {
77+
let current_hash = fingerprint_file(path);
78+
let prev = self.known_hashes.get(source).copied();
79+
80+
if prev == Some(current_hash) {
81+
return None;
82+
}
83+
84+
self.known_hashes.insert(source.to_string(), current_hash);
85+
86+
Some(SettingsChange {
87+
source: source.to_string(),
88+
changed_keys: Vec::new(), // Key-level diff is expensive; report source-level changes
89+
})
5890
}
5991

6092
/// Mark a source as known at its current state, suppressing change
6193
/// notifications until it actually changes again.
62-
pub fn mark_known(&mut self, source: &str) {
63-
todo!(
64-
"mark_known: record current fingerprint for source '{}'",
65-
source
66-
)
94+
pub fn mark_known(&mut self, source: &str, path: &Path) {
95+
let hash = fingerprint_file(path);
96+
self.known_hashes.insert(source.to_string(), hash);
97+
}
98+
99+
/// Mark a source with an explicit hash value (for non-file sources).
100+
pub fn mark_known_hash(&mut self, source: &str, hash: u64) {
101+
self.known_hashes.insert(source.to_string(), hash);
67102
}
68103
}
69104

@@ -73,6 +108,18 @@ impl Default for ChangeDetector {
73108
}
74109
}
75110

111+
/// Compute a fingerprint for a file's contents. Returns 0 if unreadable.
112+
fn fingerprint_file(path: &Path) -> u64 {
113+
match std::fs::read(path) {
114+
Ok(bytes) => {
115+
let mut hasher = DefaultHasher::new();
116+
bytes.hash(&mut hasher);
117+
hasher.finish()
118+
}
119+
Err(_) => 0, // File doesn't exist or can't be read
120+
}
121+
}
122+
76123
// ── Tests ─────────────────────────────────────────────────────────────
77124

78125
#[cfg(test)]
@@ -100,4 +147,32 @@ mod tests {
100147
assert_eq!(change.source, "global");
101148
assert_eq!(change.changed_keys.len(), 2);
102149
}
150+
151+
#[test]
152+
fn check_source_nonexistent_file() {
153+
let mut detector = ChangeDetector::new();
154+
// First check should report change (new source)
155+
let change = detector.check_source("test", Path::new("/nonexistent/path.json"));
156+
assert!(change.is_some());
157+
158+
// Second check with same (non-existent) path should not report change
159+
let change = detector.check_source("test", Path::new("/nonexistent/path.json"));
160+
assert!(change.is_none());
161+
}
162+
163+
#[test]
164+
fn mark_known_suppresses_change() {
165+
let mut detector = ChangeDetector::new();
166+
let path = Path::new("/nonexistent/path.json");
167+
detector.mark_known("test", path);
168+
169+
// Should not report change since we just marked it
170+
let change = detector.check_source("test", path);
171+
assert!(change.is_none());
172+
}
173+
174+
#[test]
175+
fn fingerprint_nonexistent_returns_zero() {
176+
assert_eq!(fingerprint_file(Path::new("/no/such/file")), 0);
177+
}
103178
}

crates/config/src/feature_flag.rs

Lines changed: 66 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,18 @@ impl FeatureFlags {
3333
///
3434
/// Expects the `"featureFlags"` key to be an object of `{ "flag_name": bool }`.
3535
/// Unknown flags are preserved. Missing flags retain their defaults.
36-
pub fn load_from_settings(_settings: &serde_json::Value) -> Self {
37-
todo!()
36+
pub fn load_from_settings(settings: &serde_json::Value) -> Self {
37+
let mut flags = HashMap::new();
38+
39+
if let Some(obj) = settings.get("featureFlags").and_then(|v| v.as_object()) {
40+
for (key, value) in obj {
41+
if let Some(b) = value.as_bool() {
42+
flags.insert(key.clone(), b);
43+
}
44+
}
45+
}
46+
47+
Self { flags }
3848
}
3949

4050
/// Check whether a flag is enabled.
@@ -55,8 +65,10 @@ impl FeatureFlags {
5565
}
5666

5767
/// Merge another set of flags on top, overriding existing values.
58-
pub fn merge(&mut self, _other: &Self) {
59-
todo!()
68+
pub fn merge(&mut self, other: &Self) {
69+
for (key, &value) in &other.flags {
70+
self.flags.insert(key.clone(), value);
71+
}
6072
}
6173
}
6274

@@ -67,27 +79,14 @@ impl Default for FeatureFlags {
6779
}
6880

6981
/// Well-known feature flag constants.
70-
///
71-
/// Use these constants when checking flags to avoid typo-related bugs:
72-
/// ```rust,ignore
73-
/// if flags.is_enabled(flags::WASM_PLUGINS) { /* ... */ }
74-
/// ```
7582
pub mod flags {
76-
/// Enable WASM plugin support.
7783
pub const WASM_PLUGINS: &str = "wasm_plugins";
78-
/// Enable MCP server authentication (`OAuth2` / API keys).
7984
pub const MCP_AUTH: &str = "mcp_auth";
80-
/// Enable shared team memory.
8185
pub const TEAM_MEMORY: &str = "team_memory";
82-
/// Enable automatic conversation compaction.
8386
pub const AUTO_COMPACT: &str = "auto_compact";
84-
/// Enable prompt suggestions in the chat input.
8587
pub const PROMPT_SUGGESTIONS: &str = "prompt_suggestions";
86-
/// Enable extended thinking display.
8788
pub const EXTENDED_THINKING: &str = "extended_thinking";
88-
/// Enable multi-turn tool use.
8989
pub const MULTI_TURN_TOOL_USE: &str = "multi_turn_tool_use";
90-
/// Enable streaming markdown rendering.
9190
pub const STREAMING_MARKDOWN: &str = "streaming_markdown";
9291
}
9392

@@ -97,10 +96,7 @@ mod tests {
9796

9897
#[test]
9998
fn unknown_flag_returns_false() {
100-
let ff = FeatureFlags {
101-
flags: HashMap::new(),
102-
};
103-
// Unknown flags should not panic, just return false.
99+
let ff = FeatureFlags::default_flags();
104100
assert!(!ff.is_enabled("nonexistent_flag"));
105101
}
106102

@@ -120,4 +116,53 @@ mod tests {
120116
assert!(!flags::AUTO_COMPACT.is_empty());
121117
assert!(!flags::PROMPT_SUGGESTIONS.is_empty());
122118
}
119+
120+
#[test]
121+
fn load_from_settings_parses_flags() {
122+
let settings = serde_json::json!({
123+
"featureFlags": {
124+
"wasm_plugins": true,
125+
"mcp_auth": false
126+
}
127+
});
128+
let ff = FeatureFlags::load_from_settings(&settings);
129+
assert!(ff.is_enabled(flags::WASM_PLUGINS));
130+
assert!(!ff.is_enabled(flags::MCP_AUTH));
131+
}
132+
133+
#[test]
134+
fn load_from_settings_no_flags_key() {
135+
let settings = serde_json::json!({"model": "claude"});
136+
let ff = FeatureFlags::load_from_settings(&settings);
137+
assert!(!ff.is_enabled(flags::WASM_PLUGINS));
138+
}
139+
140+
#[test]
141+
fn load_from_settings_ignores_non_bool() {
142+
let settings = serde_json::json!({
143+
"featureFlags": {
144+
"valid": true,
145+
"invalid": "yes"
146+
}
147+
});
148+
let ff = FeatureFlags::load_from_settings(&settings);
149+
assert!(ff.is_enabled("valid"));
150+
assert!(!ff.is_enabled("invalid"));
151+
}
152+
153+
#[test]
154+
fn merge_overrides() {
155+
let mut base = FeatureFlags::default_flags();
156+
base.set("a", true);
157+
base.set("b", false);
158+
159+
let mut overlay = FeatureFlags::default_flags();
160+
overlay.set("b", true);
161+
overlay.set("c", true);
162+
163+
base.merge(&overlay);
164+
assert!(base.is_enabled("a"));
165+
assert!(base.is_enabled("b")); // overridden
166+
assert!(base.is_enabled("c")); // new
167+
}
123168
}

0 commit comments

Comments
 (0)