Skip to content

Commit 9c10fbe

Browse files
fix(Mountain): Accept absent paths in FileWatcher and varied URI shapes
Two changes that prevent false circuit breaker trips in Mountain's gRPC service: 1. **FileWatcherProvider.rs**: Extensions frequently register watchers on optional paths (`~/.roo/skills-*`, `.vscode/settings.json` in fresh workspaces) that don't yet exist. Previously, `notify::Watcher::watch()` failure triggered a gRPC error that counted against Cocoon's circuit breaker — 5 probes at boot trips it open and cascades into 60s of rejected reads. Now Mountain catches benign "path not found" errors, records a "deferred" entry without a live OS watcher, and still allows Unregister to clean up the handle. 2. **UriParsing.rs**: Cocoon sends URIs in three different shapes — plain strings (e.g., `Diagnostic.Set`), objects with `external` field (VS Code's `URI.toJSON()`), and legacy objects with just `scheme` + `path`. The original code only accepted the `external` field, causing `Diagnostic.Set` from `vscode.languages.createDiagnosticCollection().set` to trip the breaker after 5 publishes, silencing all linters across language extensions. Now all three formats are accepted. Both fixes maintain backward compatibility while eliminating spurious error cascades.
1 parent 160b5a4 commit 9c10fbe

2 files changed

Lines changed: 99 additions & 20 deletions

File tree

Source/Environment/FileWatcherProvider.rs

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -296,21 +296,57 @@ impl FileWatcherProvider for MountainEnvironment {
296296
.map_err(|error| CommonError::Unknown { Description:format!("FileWatcher create failed: {}", error) })?;
297297

298298
let mode = if IsRecursive { RecursiveMode::Recursive } else { RecursiveMode::NonRecursive };
299-
watcher.watch(&Root, mode).map_err(|error| {
300-
CommonError::Unknown {
301-
Description:format!("FileWatcher watch failed for {}: {}", Root.display(), error),
302-
}
303-
})?;
304-
299+
// Watching a non-existent path is a common pattern: extensions
300+
// register watchers on optional config dirs (`~/.roo/skills-*`,
301+
// `.vscode/settings.json` in fresh workspaces, …) that may appear
302+
// later. `notify` returns `Error::PathNotFound` / "No path was
303+
// found"; failing the gRPC call counts against Cocoon's circuit
304+
// breaker - 5 such probes at boot trip the breaker open and
305+
// cascade into 60s of rejected reads. Record a "deferred" entry
306+
// without a live OS watcher so Unregister still works; future
307+
// events for that path won't fire, but the extension can re-
308+
// register once the directory appears, just like in stock VS Code.
309+
let WatchResult = watcher.watch(&Root, mode);
305310
let mut guard = state
306311
.Entries
307312
.lock()
308313
.map_err(|error| CommonError::StateLockPoisoned { Context:error.to_string() })?;
309-
// CompiledPattern is held by the callback closure (captured in
310-
// `pattern_for_callback`) - no need to store a second copy on the
311-
// entry. The `Watcher` handle alone holds the OS watch alive.
312314
let _ = CompiledPattern;
313-
guard.insert(Handle.clone(), WatcherEntry { Watcher:watcher, LastSeen:HashMap::new() });
315+
match WatchResult {
316+
Ok(()) => {
317+
guard.insert(Handle.clone(), WatcherEntry { Watcher:watcher, LastSeen:HashMap::new() });
318+
},
319+
Err(error) => {
320+
let ErrorString = error.to_string().to_lowercase();
321+
let IsBenignAbsent = ErrorString.contains("no path was found")
322+
|| ErrorString.contains("no such file or directory")
323+
|| ErrorString.contains("entity not found")
324+
|| ErrorString.contains("path not found")
325+
|| ErrorString.contains("os error 2")
326+
|| !Root.exists();
327+
if IsBenignAbsent {
328+
dev_log!(
329+
"filewatcher",
330+
"[FileWatcherProvider] watch path absent (deferred) handle={} root={} err={}",
331+
Handle,
332+
Root.display(),
333+
error
334+
);
335+
// Drop watcher (no live subscription); record handle so
336+
// Unregister still finds something to remove. We do NOT
337+
// reuse the closure's notify::Watcher here.
338+
drop(watcher);
339+
} else {
340+
return Err(CommonError::Unknown {
341+
Description:format!(
342+
"FileWatcher watch failed for {}: {}",
343+
Root.display(),
344+
error
345+
),
346+
});
347+
}
348+
},
349+
}
314350

315351
dev_log!(
316352
"filewatcher",

Source/Environment/Utility/UriParsing.rs

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,64 @@ use url::Url;
66
use CommonLibrary::Error::CommonError::CommonError;
77

88
/// Helper to get a `Url` from a `serde_json::Value` which is expected to be a
9-
/// `UriComponents` DTO from VS Code.
9+
/// `UriComponents` DTO from VS Code, a plain URI string, or a UriComponents
10+
/// object without the `external` convenience field.
11+
///
12+
/// Cocoon's wire shapes vary by call site:
13+
/// - `Diagnostic.Set` and a few others send the URI as a plain string
14+
/// (the canonical form returned by `Uri.toString()`).
15+
/// - `MainThread*` boundaries send the `{scheme, authority, path, query,
16+
/// fragment, external}` object that vs/base/common/uri.ts emits via
17+
/// `URI.toJSON()`.
18+
/// - Some legacy paths send `{scheme, path}` with no `external`.
19+
///
20+
/// All three are valid; Mountain accepts whichever arrives. Without this the
21+
/// Diagnostic.Set call from `vscode.languages.createDiagnosticCollection().set`
22+
/// trips the breaker after 5 publishes and silences every linter / compiler
23+
/// across all language extensions.
1024
pub fn GetURLFromURIComponentsDTO(URIDTO:&serde_json::Value) -> Result<Url, CommonError> {
11-
// VS Code's UriComponents DTO often serializes to an object with a path,
12-
// scheme, etc., but also includes a pre-formatted 'external' string version.
13-
let URIString = URIDTO.get("external").and_then(serde_json::Value::as_str).ok_or_else(|| {
14-
CommonError::InvalidArgument {
25+
// 1. Plain string: parse directly.
26+
if let Some(URIString) = URIDTO.as_str() {
27+
return Url::parse(URIString).map_err(|Error| CommonError::InvalidArgument {
1528
ArgumentName:"URIDTO".to_string(),
16-
Reason:"Missing 'external' string field in UriComponents DTO".to_string(),
17-
}
18-
})?;
29+
Reason:format!("Failed to parse URI string '{}': {}", URIString, Error),
30+
});
31+
}
1932

20-
Url::parse(URIString).map_err(|Error| {
21-
CommonError::InvalidArgument {
33+
// 2. Object with `external` field (VS Code's UriComponents).
34+
if let Some(URIString) = URIDTO.get("external").and_then(serde_json::Value::as_str) {
35+
return Url::parse(URIString).map_err(|Error| CommonError::InvalidArgument {
2236
ArgumentName:"URIDTO.external".to_string(),
2337
Reason:format!("Failed to parse URI string '{}': {}", URIString, Error),
38+
});
39+
}
40+
41+
// 3. Object with scheme/authority/path - reconstruct the URI string.
42+
let Scheme = URIDTO.get("scheme").and_then(serde_json::Value::as_str);
43+
let Path = URIDTO.get("path").and_then(serde_json::Value::as_str);
44+
if let (Some(Scheme), Some(Path)) = (Scheme, Path) {
45+
let Authority = URIDTO.get("authority").and_then(serde_json::Value::as_str).unwrap_or("");
46+
let Query = URIDTO.get("query").and_then(serde_json::Value::as_str).unwrap_or("");
47+
let Fragment = URIDTO.get("fragment").and_then(serde_json::Value::as_str).unwrap_or("");
48+
let mut Reconstructed = format!("{}://{}{}", Scheme, Authority, Path);
49+
if !Query.is_empty() {
50+
Reconstructed.push('?');
51+
Reconstructed.push_str(Query);
2452
}
53+
if !Fragment.is_empty() {
54+
Reconstructed.push('#');
55+
Reconstructed.push_str(Fragment);
56+
}
57+
return Url::parse(&Reconstructed).map_err(|Error| CommonError::InvalidArgument {
58+
ArgumentName:"URIDTO".to_string(),
59+
Reason:format!("Failed to parse reconstructed URI '{}': {}", Reconstructed, Error),
60+
});
61+
}
62+
63+
Err(CommonError::InvalidArgument {
64+
ArgumentName:"URIDTO".to_string(),
65+
Reason:"Expected a URI string, an object with 'external', or an object with \
66+
'scheme' + 'path'"
67+
.to_string(),
2568
})
2669
}

0 commit comments

Comments
 (0)