Skip to content

Commit 1999d56

Browse files
authored
fix(runtimed): handle directory paths in --attach without autosave spin loop (#1028)
* fix(runtimed): handle directory paths in --attach without autosave spin loop When `--attach <directory>` passes a directory path, detect it early in handle_open_notebook and delegate to handle_create_notebook with the directory as working_dir. Also introduces a typed SaveError enum so the autosave loop can distinguish unrecoverable errors (IsADirectory, PermissionDenied) from transient ones and break instead of retrying forever. Closes #1027 * fix(runtimed): use O_EXCL probe for directory writability check Replace the fixed-name write probe with a UUID-named file created via File::create_new (O_EXCL), so the writability check can never truncate or remove an existing user file.
1 parent 1fe0aa5 commit 1999d56

File tree

2 files changed

+82
-6
lines changed

2 files changed

+82
-6
lines changed

crates/runtimed/src/daemon.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,6 +1104,43 @@ impl Daemon {
11041104
// Check if file exists before canonicalizing (canonicalize fails for non-existent paths)
11051105
let mut path_buf = std::path::PathBuf::from(&path);
11061106
let file_exists = match tokio::fs::metadata(&path_buf).await {
1107+
Ok(meta) if meta.is_dir() => {
1108+
// Directory path — create untitled notebook with this as working dir
1109+
info!(
1110+
"[runtimed] Path {} is a directory, creating untitled notebook with working_dir",
1111+
path
1112+
);
1113+
let dir_path = match path_buf.canonicalize() {
1114+
Ok(p) => p.to_string_lossy().to_string(),
1115+
Err(_) => path_buf.to_string_lossy().to_string(),
1116+
};
1117+
// Verify directory is writable using a uniquely-named temp file.
1118+
// create_new uses O_EXCL so it can never clobber an existing file.
1119+
let probe = path_buf.join(format!(".runtimed_probe_{}", uuid::Uuid::new_v4()));
1120+
match std::fs::File::create_new(&probe) {
1121+
Ok(_) => {
1122+
let _ = std::fs::remove_file(&probe);
1123+
}
1124+
Err(e) => {
1125+
let (_reader, mut writer) = tokio::io::split(stream);
1126+
send_error_response(
1127+
&mut writer,
1128+
format!("Directory '{}' is not writable: {}", path, e),
1129+
)
1130+
.await?;
1131+
return Ok(());
1132+
}
1133+
}
1134+
let settings = self.settings.read().await.get_all();
1135+
return self
1136+
.handle_create_notebook(
1137+
stream,
1138+
settings.default_runtime.to_string(),
1139+
Some(dir_path),
1140+
None,
1141+
)
1142+
.await;
1143+
}
11071144
Ok(_) => true,
11081145
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
11091146
// For new files, ensure .ipynb extension

crates/runtimed/src/notebook_sync_server.rs

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3772,11 +3772,28 @@ async fn format_notebook_cells(room: &NotebookRoom) -> Result<usize, String> {
37723772
/// 4. Reconstruct cells: source and outputs from Automerge, cell metadata from existing file
37733773
/// 5. Write the merged notebook to disk
37743774
///
3775+
/// Errors from save_notebook_to_disk.
3776+
#[derive(Debug)]
3777+
enum SaveError {
3778+
/// Transient / potentially recoverable (e.g. disk full, busy)
3779+
Retryable(String),
3780+
/// Permanent — retrying will never help (path is a directory, permission denied, invalid path)
3781+
Unrecoverable(String),
3782+
}
3783+
3784+
impl std::fmt::Display for SaveError {
3785+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
3786+
match self {
3787+
SaveError::Retryable(msg) | SaveError::Unrecoverable(msg) => f.write_str(msg),
3788+
}
3789+
}
3790+
}
3791+
37753792
/// Returns the absolute path where the notebook was written.
37763793
async fn save_notebook_to_disk(
37773794
room: &NotebookRoom,
37783795
target_path: Option<&str>,
3779-
) -> Result<String, String> {
3796+
) -> Result<String, SaveError> {
37803797
// Determine the actual save path
37813798
let notebook_path = match target_path {
37823799
Some(p) => {
@@ -3785,10 +3802,10 @@ async fn save_notebook_to_disk(
37853802
// Reject relative paths - daemon CWD is unpredictable (could be / when running as launchd)
37863803
// Clients (Tauri file dialog, Python SDK) should always provide absolute paths.
37873804
if path.is_relative() {
3788-
return Err(format!(
3805+
return Err(SaveError::Unrecoverable(format!(
37893806
"Relative paths are not supported for save: '{}'. Please provide an absolute path.",
37903807
p
3791-
));
3808+
)));
37923809
}
37933810

37943811
// Ensure .ipynb extension
@@ -3918,13 +3935,21 @@ async fn save_notebook_to_disk(
39183935

39193936
// Serialize with trailing newline (nbformat convention)
39203937
let content = serde_json::to_string_pretty(&notebook_json)
3921-
.map_err(|e| format!("Failed to serialize notebook: {e}"))?;
3938+
.map_err(|e| SaveError::Retryable(format!("Failed to serialize notebook: {e}")))?;
39223939
let content_with_newline = format!("{content}\n");
39233940

39243941
// Write to disk (async to avoid blocking the runtime)
39253942
tokio::fs::write(&notebook_path, content_with_newline)
39263943
.await
3927-
.map_err(|e| format!("Failed to write notebook: {e}"))?;
3944+
.map_err(|e| {
3945+
let msg = format!("Failed to write notebook: {e}");
3946+
match e.kind() {
3947+
std::io::ErrorKind::PermissionDenied | std::io::ErrorKind::IsADirectory => {
3948+
SaveError::Unrecoverable(msg)
3949+
}
3950+
_ => SaveError::Retryable(msg),
3951+
}
3952+
})?;
39283953

39293954
// Update last_self_write timestamp so file watcher skips this change
39303955
let now = std::time::SystemTime::now()
@@ -4340,6 +4365,13 @@ fn spawn_autosave_debouncer_with_config(
43404365
);
43414366
}
43424367
}
4368+
Err(ref e @ SaveError::Unrecoverable(_)) => {
4369+
error!(
4370+
"[autosave] Unrecoverable save error, disabling autosave for {}: {}",
4371+
notebook_id, e
4372+
);
4373+
break;
4374+
}
43434375
Err(e) => {
43444376
warn!("[autosave] Failed to save: {}", e);
43454377
// Keep last_receive set so we retry on next interval
@@ -6901,7 +6933,14 @@ mod tests {
69016933
assert!(result.is_err());
69026934
let error = result.unwrap_err();
69036935
assert!(
6904-
error.contains("Relative paths are not supported"),
6936+
matches!(error, SaveError::Unrecoverable(_)),
6937+
"Error should be unrecoverable: {}",
6938+
error
6939+
);
6940+
assert!(
6941+
error
6942+
.to_string()
6943+
.contains("Relative paths are not supported"),
69056944
"Error should mention relative paths: {}",
69066945
error
69076946
);

0 commit comments

Comments
 (0)