Skip to content

Desktop: Add support for opening files through the already-running Graphite executable on Windows and Linux#4117

Open
Keavon wants to merge 3 commits intomasterfrom
file-open-ipc-windows-linux
Open

Desktop: Add support for opening files through the already-running Graphite executable on Windows and Linux#4117
Keavon wants to merge 3 commits intomasterfrom
file-open-ipc-windows-linux

Conversation

@Keavon
Copy link
Copy Markdown
Member

@Keavon Keavon commented May 6, 2026

No description provided.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 8 files

Confidence score: 2/5

  • I’m rating this as high risk because desktop/src/instance_ipc.rs uses from_encoded_bytes_unchecked on IPC-provided bytes, and that violates the function’s documented safety contract.
  • The reported impact is concrete: a local process can send malformed bytes and potentially trigger undefined behavior, which is a serious stability/security concern.
  • Confidence is reasonably strong (severity 8/10, confidence 7/10), so this looks like a real merge blocker rather than a speculative edge case.
  • Pay close attention to desktop/src/instance_ipc.rs - unchecked reconstruction from untrusted IPC bytes can lead to undefined behavior.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="desktop/src/instance_ipc.rs">

<violation number="1" location="desktop/src/instance_ipc.rs:143">
P1: `from_encoded_bytes_unchecked` is used on IPC-provided bytes, but its safety contract explicitly forbids reconstructing from arbitrary/network-sent bytes. A local process can send invalid data and trigger undefined behavior. Prefer a safe encoding (e.g., UTF-8) with validation or otherwise validate the bytes before conversion.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread desktop/src/instance_ipc.rs Outdated
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements single-instance file-open handoff for Windows and Linux, allowing a secondary process to forward file paths to a running instance via local IPC. It introduces the interprocess crate and a new instance_ipc module to handle communication. Feedback was provided regarding the importance of handling the result of thread spawning in the IPC listener to prevent silent failures.

Comment on lines +79 to +93
let _ = thread::Builder::new().name("graphite-instance-ipc".into()).spawn(move || {
for connection in listener.incoming() {
match connection {
Ok(mut stream) => match read_paths(&mut stream) {
Ok(paths) if !paths.is_empty() => {
tracing::info!("Received {} file path(s) from secondary instance", paths.len());
scheduler.schedule(AppEvent::AddLaunchDocuments(paths));
}
Ok(_) => {}
Err(error) => tracing::error!("Failed to read paths from secondary instance: {error}"),
},
Err(error) => tracing::error!("Instance IPC accept failed: {error}"),
}
}
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The spawn method on thread::Builder returns a Result which is marked with #[must_use]. Ignoring it will cause a compiler warning and means that if the thread fails to spawn, the error will be silently ignored. It's better to handle this potential error, for example by logging it.

	if let Err(error) = thread::Builder::new().name("graphite-instance-ipc".into()).spawn(move || {
		for connection in listener.incoming() {
			match connection {
				Ok(mut stream) => match read_paths(&mut stream) {
					Ok(paths) if !paths.is_empty() => {
						tracing::info!("Received {} file path(s) from secondary instance", paths.len());
						scheduler.schedule(AppEvent::AddLaunchDocuments(paths));
					}
					Ok(_) => {}
					Err(error) => tracing::error!("Failed to read paths from secondary instance: {error}"),
				},
				Err(error) => tracing::error!("Instance IPC accept failed: {error}"),
			}
		}
	}) {
		tracing::error!("Failed to spawn instance IPC listener thread: {error}");
	}

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.

1 participant