Skip to content

Commit 53d0c4e

Browse files
committed
feat(mcp): hand images to an artifact sink as well as to the model
A screenshot returned from browser_take_screenshot reaches exactly one place: the model's context. That is enough for the model to look at the page and no use at all to the person who asked to see it — they get a description of an image that was never sent anywhere. The agent cannot fix that on its own either, because the bytes never touch a filesystem it can reach. So when RUSTWRIGHT_ARTIFACT_SINK names an endpoint, every image this server produces is POSTed there as well. The operator decides what that means. It is best effort throughout: the tool call's real job is answering the model, and a sink that is missing, slow or broken must not turn a working screenshot into a failed tool. Placed before the inline size branch deliberately — an image too large to inline is the one a person most wants sent somewhere they can open it, and telling the model "saved to /tmp/..." helps nobody outside the machine. Written by hand over a unix socket rather than pulling in an HTTP client for a request with a fixed shape sent to the same host.
1 parent 9d738c4 commit 53d0c4e

3 files changed

Lines changed: 167 additions & 0 deletions

File tree

mcp/src/artifacts.rs

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
//! Handing media the browser produced to whoever is hosting this server.
2+
//!
3+
//! # Why this exists
4+
//!
5+
//! A screenshot returned from `browser_take_screenshot` reaches exactly one
6+
//! place: the model's context. That is enough for the model to *look* at the
7+
//! page and no use at all to the person who asked to see it — they get a
8+
//! description of an image that was never sent anywhere. The agent has no way
9+
//! to fix that on its own either, because the bytes never touch a filesystem it
10+
//! can reach.
11+
//!
12+
//! So when an operator points [`ENV_SINK`] at an endpoint, every image this
13+
//! server produces is also POSTed there. The operator decides what that means:
14+
//! attach it to a chat message, drop it in a bucket, ignore it.
15+
//!
16+
//! # Best effort, always
17+
//!
18+
//! Every failure here is swallowed. The tool call's real job is answering the
19+
//! model, and a sink that is missing, slow, or broken must not turn a working
20+
//! screenshot into a failed tool. Nothing in this module returns an error to a
21+
//! caller; it logs to stderr and moves on.
22+
23+
use std::io::{Read, Write};
24+
use std::os::unix::net::UnixStream;
25+
use std::time::Duration;
26+
27+
/// Where to POST media this server produces.
28+
///
29+
/// A URL rather than a bare path, and read from the environment rather than
30+
/// configured in a tool call: whether artifacts are collected is a property of
31+
/// the deployment, not something a model should be deciding per screenshot.
32+
///
33+
/// Only `unix://` is understood today. An operator running this somewhere else
34+
/// gets a warning on the first attempt and no artifacts, which is the same
35+
/// outcome as not setting it.
36+
pub const ENV_SINK: &str = "RUSTWRIGHT_ARTIFACT_SINK";
37+
38+
/// How long one POST may take.
39+
///
40+
/// The tool call is holding while this runs, so it is short. A sink that cannot
41+
/// take a screenshot in this long is one whose artifacts are not worth the
42+
/// latency they are costing the person waiting for an answer.
43+
const TIMEOUT: Duration = Duration::from_secs(20);
44+
45+
/// Refuse to hold more than this in one request.
46+
///
47+
/// Well above any screenshot and below the point where a stuck encoder could
48+
/// have us buffering a video into memory forever.
49+
const MAX_BYTES: usize = 96 * 1024 * 1024;
50+
51+
/// Post one artifact, if a sink is configured.
52+
///
53+
/// `name` is a suggestion. A sink is free to rename or ignore it — it reaches a
54+
/// header, so it is sanitized here rather than trusted.
55+
pub fn offer(bytes: &[u8], mime: &str, name: &str) {
56+
let Ok(url) = std::env::var(ENV_SINK) else {
57+
return;
58+
};
59+
let url = url.trim();
60+
if url.is_empty() {
61+
return;
62+
}
63+
let Some(path) = url.strip_prefix("unix://") else {
64+
eprintln!("[rustwright] {ENV_SINK} is not a unix:// URL; ignoring it");
65+
return;
66+
};
67+
if bytes.is_empty() || bytes.len() > MAX_BYTES {
68+
return;
69+
}
70+
71+
if let Err(error) = post(path, bytes, mime, name) {
72+
// Worth saying out loud: from the model's side the tool succeeded, so a
73+
// silent failure here is a screenshot that vanishes with no trace.
74+
eprintln!("[rustwright] could not hand an artifact to the sink: {error}");
75+
}
76+
}
77+
78+
/// Write one HTTP/1.1 POST by hand and read back the status line.
79+
///
80+
/// By hand because the alternative is an HTTP client dependency for a request
81+
/// with a fixed shape, sent to a socket on the same machine. Content-Length,
82+
/// not chunked: the length is known and a sink that streams straight into
83+
/// object storage wants it up front.
84+
fn post(path: &str, bytes: &[u8], mime: &str, name: &str) -> std::io::Result<()> {
85+
let mut stream = UnixStream::connect(path)?;
86+
stream.set_write_timeout(Some(TIMEOUT))?;
87+
stream.set_read_timeout(Some(TIMEOUT))?;
88+
89+
let head = format!(
90+
"POST / HTTP/1.1\r\n\
91+
Host: localhost\r\n\
92+
Content-Type: {}\r\n\
93+
X-Artifact-Name: {}\r\n\
94+
Content-Length: {}\r\n\
95+
Connection: close\r\n\r\n",
96+
header_safe(mime),
97+
header_safe(name),
98+
bytes.len(),
99+
);
100+
stream.write_all(head.as_bytes())?;
101+
stream.write_all(bytes)?;
102+
stream.flush()?;
103+
104+
// Enough for the status line. The body is not interesting and the
105+
// connection closes either way.
106+
let mut response = [0u8; 64];
107+
let read = stream.read(&mut response).unwrap_or(0);
108+
let status = String::from_utf8_lossy(&response[..read]);
109+
let ok = status
110+
.split_whitespace()
111+
.nth(1)
112+
.and_then(|code| code.parse::<u16>().ok())
113+
.is_some_and(|code| (200..300).contains(&code));
114+
if !ok {
115+
return Err(std::io::Error::other(format!(
116+
"sink refused it: {}",
117+
status.lines().next().unwrap_or("no response").trim()
118+
)));
119+
}
120+
Ok(())
121+
}
122+
123+
/// Strip anything that would end a header line or smuggle another one in.
124+
///
125+
/// These values are ours rather than a user's, but `name` is derived from a
126+
/// page title in some callers and a CR in a header is a request-splitting bug
127+
/// wherever it comes from.
128+
fn header_safe(value: &str) -> String {
129+
value
130+
.chars()
131+
.filter(|c| !c.is_control())
132+
.take(200)
133+
.collect()
134+
}
135+
136+
#[cfg(test)]
137+
mod tests {
138+
use super::*;
139+
140+
#[test]
141+
fn header_safe_drops_control_characters() {
142+
assert_eq!(header_safe("a\r\nX-Evil: 1"), "aX-Evil: 1");
143+
assert_eq!(header_safe("shot.png"), "shot.png");
144+
}
145+
146+
#[test]
147+
fn header_safe_bounds_length() {
148+
assert_eq!(header_safe(&"x".repeat(500)).len(), 200);
149+
}
150+
151+
// No sink configured is the common case and must be free of side effects.
152+
#[test]
153+
fn offer_without_a_sink_does_nothing() {
154+
// SAFETY: single-threaded test, and the variable is read once per call.
155+
unsafe { std::env::remove_var(ENV_SINK) };
156+
offer(b"not a real png", "image/png", "shot.png");
157+
}
158+
}

mcp/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
mod actor;
2+
mod artifacts;
23
mod config;
34
mod server;
45
mod shaping;

mcp/src/server.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,14 @@ fn output_content(
163163
mime,
164164
extension,
165165
} => {
166+
// Hand it to the host as well as to the model.
167+
//
168+
// Before the size branch below, deliberately: an image too large to
169+
// inline is exactly the one a person most wants sent somewhere they
170+
// can open it, and telling the model "saved to /tmp/..." helps
171+
// nobody outside this VM.
172+
crate::artifacts::offer(&bytes, &mime, &format!("screenshot.{extension}"));
173+
166174
let payload_bytes = encoded_len(bytes.len(), true).unwrap_or(usize::MAX);
167175
if payload_bytes <= screenshot_max_bytes {
168176
return Ok((ContentBlock::image(STANDARD.encode(bytes), mime), None));

0 commit comments

Comments
 (0)