supercode_harness/

formatters.rs

//! §2 module 29 `formatters` (COMPOSABLE-HARNESS-DESIGN.md line 479):
//! "D10/oc§10 format-on-write" — reuses the EXACT [`crate::tools::WriteObserver`]
//! seam P5-9 built for `checkpoint` (D-5: "write-path interception seam
//! shared with checkpoint"), rather than a second interception point.
//!
//! # C10 (design line 534) — the critical correctness rule
//! "Post-write formatting invalidates the model's file memory; formatter
//! must diff-back into the result (oc§10)." Concretely: once a formatter
//! rewrites a file the model just wrote/edited, the model's IN-CONTEXT
//! belief about that file's bytes is stale. `FormatObserver::after_write`
//! (when `[capabilities.formatters] diff_back = true`, the default) returns
//! a unified diff of exactly what the formatter changed, appended to the
//! calling tool's result — so the model's next action is informed by the
//! ACTUAL on-disk bytes, not its own pre-format draft. `diff_back = false`
//! still runs the formatter (the file changes) but withholds the
//! annotation — the C10-UNSAFE mode, legal but never the default.
//!
//! # Wire model — stdin -> stdout filter
//! A configured formatter is invoked as `command args...` with the
//! JUST-WRITTEN file's bytes piped to its stdin; its stdout (bounded,
//! [`MAX_FORMATTER_OUTPUT_BYTES`]) becomes the new file content IF the
//! process exits `0` and produces non-empty output that differs from the
//! input. This is the standard "formatter as filter" contract real tools
//! already support in this mode (`gofmt` reads stdin/writes stdout by
//! default; `rustfmt --emit stdout`; `prettier --stdin-filepath <name>`;
//! `black -`), and it needs no `%f`-style path-templating in the config
//! schema — the weakest form that still composes with arbitrary real
//! formatters. A non-zero exit, a timeout, or empty output is treated as
//! "formatter had nothing useful to say" and never corrupts the file: the
//! on-disk content from the write/edit tool is left exactly as that tool
//! produced it.
//!
//! # Ordering (composes with `checkpoint` + `lsp` on the shared seam)
//! `crate::agent::build_tool_context` installs observers in the order
//! `checkpoint -> formatters -> lsp` via
//! [`crate::tools::WriteObserverChain`]: checkpoint's `before_write`
//! captures the pre-image before ANY mutation; this module's `after_write`
//! reformats the just-written file; `lsp`'s `after_write` (running AFTER
//! this one in the same chain) then reads the FINAL, formatted file for
//! diagnostics — never the model's pre-format draft.

use std::path::{Path, PathBuf};
use std::time::Duration;

use tokio::io::{AsyncReadExt, AsyncWriteExt};

/// Bound on a single formatter invocation's stdout — a hostile or broken
/// configured formatter can't force unbounded memory growth (same
/// rationale as `crate::mcp::MCP_MAX_RESPONSE_BYTES`).
pub const MAX_FORMATTER_OUTPUT_BYTES: usize = 8 * 1024 * 1024;

/// Bound on the unified diff text appended to a tool result — a formatter
/// that rewrites a huge file can't blow the model's context with a huge
/// diff either (same bounded-annotation posture as `crate::lsp`'s
/// diagnostics cap).
pub const MAX_DIFF_CHARS: usize = 6000;

/// Default per-invocation timeout — a hanging formatter can't hang the
/// write-path loop (build brief: "timeout + kill like hooks").
pub const DEFAULT_FORMATTER_TIMEOUT_SECS: u64 = 10;

/// One `[capabilities.formatters.<name>]` entry — a user-configured
/// formatter command, invoked as a stdin->stdout filter (see module doc
/// comment). `command`/`args` are config-borne code execution (D-10) —
/// stripped from an untrusted project layer exactly like
/// `[capabilities.lsp.servers.*]`/`hooks`/`mcp.servers`.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct FormatterSpec {
    /// The executable to spawn.
    pub command: String,
    /// Extra arguments passed to `command`.
    pub args: Vec<String>,
    /// File extensions (with or without a leading `.`, matched case-
    /// insensitively) this formatter handles.
    pub extensions: Vec<String>,
}

fn spec_for_extension<'a>(
    specs: &'a [(String, FormatterSpec)],
    path: &Path,
) -> Option<&'a (String, FormatterSpec)> {
    let ext = path.extension()?.to_str()?.to_ascii_lowercase();
    specs.iter().find(|(_, s)| {
        s.extensions
            .iter()
            .any(|e| e.trim_start_matches('.').to_ascii_lowercase() == ext)
    })
}

/// Run `spec` as a stdin->stdout filter over `input`, bounded by `timeout`
/// (wall clock) and [`MAX_FORMATTER_OUTPUT_BYTES`] (output size). Returns
/// `Ok(None)` for any "formatter had nothing useful to say" outcome (never
/// an `Err` the caller has to specially handle to stay safe) — `Err` is
/// reserved for a spawn failure, which the caller logs then also treats as
/// "leave the file alone".
async fn run_formatter(
    spec: &FormatterSpec,
    input: &[u8],
    timeout: Duration,
) -> crate::error::Result<Option<Vec<u8>>> {
    // Same grandchild-orphan posture as `crate::lsp::LspClient::spawn`
    // (P5-11 review): put the formatter in its OWN process group so a
    // timed-out/hung formatter that has already spawned a helper process
    // can be group-killed below, not just its direct pid. Lower risk here
    // than LSP (a formatter is a short-lived stdin->stdout filter, not a
    // persistent server with its own worker subprocesses), but the primitive
    // is nearly free to apply consistently.
    let mut cmd = tokio::process::Command::new(&spec.command);
    cmd.args(&spec.args)
        .stdin(std::process::Stdio::piped())
        .stdout(std::process::Stdio::piped())
        .stderr(std::process::Stdio::null())
        .kill_on_drop(true);
    #[cfg(unix)]
    cmd.process_group(0);
    let mut child = cmd.spawn().map_err(|e| {
        crate::error::Error::tool("formatters", format!("spawn {}: {e}", spec.command))
    })?;
    #[cfg(unix)]
    let child_pid = child.id();

    let mut stdin = child
        .stdin
        .take()
        .ok_or_else(|| crate::error::Error::tool("formatters", "no stdin"))?;
    let mut stdout = child
        .stdout
        .take()
        .ok_or_else(|| crate::error::Error::tool("formatters", "no stdout"))?;

    let owned_input = input.to_vec();
    // Write on a separate task so a formatter that starts emitting output
    // before it has consumed all of stdin can never deadlock this process
    // against a full OS pipe buffer in either direction.
    let writer = tokio::spawn(async move {
        let _ = stdin.write_all(&owned_input).await;
        // `stdin` drops here, closing the pipe — EOF for the child.
    });
    let reader = tokio::spawn(async move {
        let mut buf = Vec::new();
        let mut limited = (&mut stdout).take(MAX_FORMATTER_OUTPUT_BYTES as u64);
        let _ = limited.read_to_end(&mut buf).await;
        buf
    });

    let wait_result = tokio::time::timeout(timeout, child.wait()).await;
    match wait_result {
        Ok(Ok(status)) => {
            writer.abort();
            let output = reader.await.unwrap_or_default();
            if !status.success() {
                return Ok(None); // non-zero exit: leave the file untouched
            }
            if output.is_empty() {
                return Ok(None); // no output: nothing to apply
            }
            Ok(Some(output))
        }
        Ok(Err(e)) => Err(crate::error::Error::tool(
            "formatters",
            format!("wait failed: {e}"),
        )),
        Err(_elapsed) => {
            // Timeout: explicitly SIGKILL the formatter's WHOLE process
            // group (unix) — same mechanism as `crate::lsp::LspClient::kill`
            // / `crate::agent::kill_job_process_group` — so a hung
            // formatter that already spawned a helper process doesn't leave
            // it running past this timeout. `child` itself (still owned by
            // this function's stack, never moved into the timed-out
            // `child.wait()` future) is then dropped when this function
            // returns — `.kill_on_drop(true)` reaps the direct pid as a
            // second, independent backstop. Abort the reader/writer tasks
            // too so they don't linger against a since-killed process's
            // now-closed pipes.
            #[cfg(unix)]
            if let Some(pid) = child_pid {
                crate::lsp::kill_process_group(pid);
            }
            writer.abort();
            reader.abort();
            Err(crate::error::Error::tool(
                "formatters",
                format!("timed out after {:?}", timeout),
            ))
        }
    }
}

/// The [`crate::tools::WriteObserver`] `[capabilities.formatters]`
/// installs. `before_write` is a true no-op (format-on-write only ever
/// acts AFTER a mutation). `after_write` runs the configured formatter (if
/// any matches `path`'s extension), rewrites the file when the formatter's
/// output differs from what was just written, and — when
/// `Self::diff_back` is `true` — returns a unified diff annotation so
/// the calling tool's result stays truthful about the file's final bytes
/// (C10).
#[derive(Debug)]
pub struct FormatObserver {
    specs: Vec<(String, FormatterSpec)>,
    root: PathBuf,
    timeout: Duration,
    diff_back: bool,
}

impl FormatObserver {
    /// Build an observer over `specs` (name -> formatter definition),
    /// rooted at `root` (the containment floor every touched path is
    /// checked against via `crate::safe_path::contained`).
    pub fn new(
        root: PathBuf,
        specs: Vec<(String, FormatterSpec)>,
        timeout: Duration,
        diff_back: bool,
    ) -> Self {
        FormatObserver {
            specs,
            root,
            timeout,
            diff_back,
        }
    }
}

#[async_trait::async_trait]
impl crate::tools::WriteObserver for FormatObserver {
    async fn before_write(&self, _path: &Path) {}

    async fn after_write(&self, path: &Path) -> Option<String> {
        if !crate::safe_path::contained(&self.root, path) {
            return None; // out of this module's scope — never touch outside the project
        }
        let (name, spec) = spec_for_extension(&self.specs, path)?;
        let original = match tokio::fs::read(path).await {
            Ok(b) => b,
            Err(_) => return None, // deleted/unreadable — nothing to format
        };
        let formatted = match run_formatter(spec, &original, self.timeout).await {
            Ok(Some(bytes)) => bytes,
            Ok(None) => return None, // no-op outcome (failure/timeout/empty/unchanged)
            Err(e) => {
                tracing::warn!(formatter = %name, "formatters: {e} — leaving file untouched");
                return None;
            }
        };
        if formatted == original {
            return None; // already-formatted — nothing to report
        }
        // Re-check containment on write-back too — defense in depth,
        // mirrors `crate::lsp`'s symmetric posture (the path hasn't
        // changed since the check above, but the cost of re-checking is
        // negligible and it keeps this function's own invariant local
        // rather than relying solely on the caller).
        if !crate::safe_path::contained(&self.root, path) {
            return None;
        }
        if tokio::fs::write(path, &formatted).await.is_err() {
            tracing::warn!(formatter = %name, path = %path.display(), "formatters: failed to write formatted output");
            return None;
        }
        if !self.diff_back {
            // C10-unsafe mode: the file changed, but the model isn't told
            // — legal (build brief: "allowed but it's the non-default"),
            // never the default (`diff_back = true`).
            return None;
        }
        let original_text = String::from_utf8_lossy(&original);
        let formatted_text = String::from_utf8_lossy(&formatted);
        let mut diff = diffy::create_patch(&original_text, &formatted_text).to_string();
        if diff.chars().count() > MAX_DIFF_CHARS {
            diff = diff.chars().take(MAX_DIFF_CHARS).collect::<String>();
            diff.push_str("\n... (diff truncated)");
        }
        let display_path = path.strip_prefix(&self.root).unwrap_or(path);
        Some(format!(
            "Formatter `{name}` reformatted {} — diff:\n{diff}",
            display_path.display()
        ))
    }
}

/// Build the [`FormatObserver`] a fresh [`crate::Agent`] should install,
/// given a resolved [`crate::Config`] — called once, from
/// `crate::agent::build_tool_context`. `Config::formatters_enabled` is the
/// ONE gate: `false` (the default) returns `None` — no formatter ever
/// runs, byte-identical to before this module existed.
pub fn observer_for_config(config: &crate::Config) -> Option<std::sync::Arc<FormatObserver>> {
    if !config.formatters_enabled {
        return None;
    }
    if config.formatters.is_empty() {
        eprintln!(
            "warning: [capabilities.formatters] is enabled but no formatters are configured \
             under [capabilities.formatters.<name>] — nothing will ever be reformatted"
        );
    }
    Some(std::sync::Arc::new(FormatObserver::new(
        config.cwd.clone(),
        config.formatters.clone(),
        Duration::from_secs(config.formatters_timeout_secs.max(1)),
        config.formatters_diff_back,
    )))
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::tools::WriteObserver;

    fn tmp(tag: &str) -> PathBuf {
        let dir = std::env::temp_dir().join(format!(
            "supercode-formatters-test-{tag}-{}-{}",
            std::process::id(),
            std::time::SystemTime::now()
                .duration_since(std::time::UNIX_EPOCH)
                .unwrap()
                .as_nanos()
        ));
        std::fs::create_dir_all(&dir).unwrap();
        dir
    }

    /// A deterministic, harmless fake formatter: uppercases its stdin —
    /// never a real formatter binary, never network access (live-agent-
    /// safety, build brief).
    fn uppercase_spec() -> FormatterSpec {
        FormatterSpec {
            command: "sh".to_string(),
            args: vec!["-c".to_string(), "tr 'a-z' 'A-Z'".to_string()],
            extensions: vec![".txt".to_string()],
        }
    }

    /// A fake formatter that always exits non-zero without touching stdout
    /// — proves a failing formatter never corrupts the file.
    fn failing_spec() -> FormatterSpec {
        FormatterSpec {
            command: "sh".to_string(),
            args: vec!["-c".to_string(), "exit 1".to_string()],
            extensions: vec![".txt".to_string()],
        }
    }

    /// A fake formatter that never exits — proves the timeout bound.
    fn hanging_spec() -> FormatterSpec {
        FormatterSpec {
            command: "sh".to_string(),
            args: vec!["-c".to_string(), "cat >/dev/null; sleep 3600".to_string()],
            extensions: vec![".txt".to_string()],
        }
    }

    #[tokio::test]
    async fn observer_for_config_is_none_when_disabled_default_off_byte_identity() {
        let config = crate::Config::builder().model("m").build();
        assert!(!config.formatters_enabled);
        assert!(observer_for_config(&config).is_none());
    }

    /// C10 diff-back proof: the model writes unformatted content, the
    /// formatter reformats it, and the annotation the tool result carries
    /// reflects the FORMATTED content (not the model's raw input).
    #[tokio::test]
    async fn diff_back_true_surfaces_the_formatted_content_in_the_annotation() {
        let project = tmp("diffback-on");
        let file = project.join("f.txt");
        std::fs::write(&file, "hello world\n").unwrap();
        let observer = FormatObserver::new(
            project.clone(),
            vec![("upper".to_string(), uppercase_spec())],
            Duration::from_secs(5),
            true, // diff_back
        );
        let note = observer.after_write(&file).await;
        let note = note.expect("diff_back=true must annotate a formatting change");
        assert!(note.contains("upper"), "{note}");
        assert!(
            note.contains("HELLO WORLD"),
            "annotation must reflect the FORMATTED content, not the raw model input: {note}"
        );
        assert!(
            note.contains("-hello world") && note.contains("+HELLO WORLD"),
            "diff must show the raw input removed and the formatted output added: {note}"
        );
        let on_disk = std::fs::read_to_string(&file).unwrap();
        assert_eq!(
            on_disk, "HELLO WORLD\n",
            "the file itself must be reformatted"
        );
        std::fs::remove_dir_all(&project).ok();
    }

    /// The `diff_back = false` (C10-unsafe, non-default) mode: the file is
    /// still reformatted, but no annotation is returned.
    #[tokio::test]
    async fn diff_back_false_reformats_silently() {
        let project = tmp("diffback-off");
        let file = project.join("f.txt");
        std::fs::write(&file, "hello world\n").unwrap();
        let observer = FormatObserver::new(
            project.clone(),
            vec![("upper".to_string(), uppercase_spec())],
            Duration::from_secs(5),
            false, // diff_back
        );
        let note = observer.after_write(&file).await;
        assert!(
            note.is_none(),
            "diff_back=false must not annotate, even though the file changed: {note:?}"
        );
        let on_disk = std::fs::read_to_string(&file).unwrap();
        assert_eq!(
            on_disk, "HELLO WORLD\n",
            "the formatter must still have run and rewritten the file"
        );
        std::fs::remove_dir_all(&project).ok();
    }

    #[tokio::test]
    async fn an_already_formatted_file_produces_no_annotation_or_rewrite() {
        let project = tmp("idempotent");
        let file = project.join("f.txt");
        std::fs::write(&file, "HELLO WORLD\n").unwrap();
        let observer = FormatObserver::new(
            project.clone(),
            vec![("upper".to_string(), uppercase_spec())],
            Duration::from_secs(5),
            true,
        );
        let mtime_before = std::fs::metadata(&file).unwrap().modified().unwrap();
        std::thread::sleep(Duration::from_millis(10));
        let note = observer.after_write(&file).await;
        assert!(note.is_none());
        let mtime_after = std::fs::metadata(&file).unwrap().modified().unwrap();
        assert_eq!(
            mtime_before, mtime_after,
            "an already-formatted file must not be rewritten"
        );
        std::fs::remove_dir_all(&project).ok();
    }

    #[tokio::test]
    async fn a_failing_formatter_never_corrupts_the_file() {
        let project = tmp("failing");
        let file = project.join("f.txt");
        std::fs::write(&file, "hello world\n").unwrap();
        let observer = FormatObserver::new(
            project.clone(),
            vec![("broken".to_string(), failing_spec())],
            Duration::from_secs(5),
            true,
        );
        let note = observer.after_write(&file).await;
        assert!(note.is_none());
        let on_disk = std::fs::read_to_string(&file).unwrap();
        assert_eq!(
            on_disk, "hello world\n",
            "a failing formatter must leave the file untouched"
        );
        std::fs::remove_dir_all(&project).ok();
    }

    /// Bounded: a hanging formatter must degrade within the configured
    /// timeout, never hang the write path.
    #[tokio::test]
    async fn a_hanging_formatter_degrades_within_the_timeout_bound() {
        let project = tmp("hanging");
        let file = project.join("f.txt");
        std::fs::write(&file, "hello world\n").unwrap();
        let observer = FormatObserver::new(
            project.clone(),
            vec![("hangs".to_string(), hanging_spec())],
            Duration::from_millis(500),
            true,
        );
        let started = std::time::Instant::now();
        let note = tokio::time::timeout(Duration::from_secs(10), observer.after_write(&file))
            .await
            .expect("must not hang past the configured formatter timeout");
        assert!(note.is_none());
        assert!(
            started.elapsed() < Duration::from_secs(5),
            "took {:?}, expected to bail out near the 500ms configured timeout",
            started.elapsed()
        );
        let on_disk = std::fs::read_to_string(&file).unwrap();
        assert_eq!(
            on_disk, "hello world\n",
            "a timed-out formatter must leave the file untouched"
        );
        std::fs::remove_dir_all(&project).ok();
    }

    #[tokio::test]
    async fn an_unconfigured_extension_is_a_true_noop() {
        let project = tmp("unconfigured");
        let file = project.join("f.py");
        std::fs::write(&file, "hello world\n").unwrap();
        let observer = FormatObserver::new(
            project.clone(),
            vec![("upper".to_string(), uppercase_spec())], // only .txt
            Duration::from_secs(5),
            true,
        );
        let note = observer.after_write(&file).await;
        assert!(note.is_none());
        let on_disk = std::fs::read_to_string(&file).unwrap();
        assert_eq!(on_disk, "hello world\n");
        std::fs::remove_dir_all(&project).ok();
    }

    #[tokio::test]
    async fn a_path_outside_the_root_is_refused() {
        let project = tmp("outside-project");
        let outside = tmp("outside-elsewhere");
        let victim = outside.join("victim.txt");
        std::fs::write(&victim, "hello world\n").unwrap();
        let observer = FormatObserver::new(
            project.clone(),
            vec![("upper".to_string(), uppercase_spec())],
            Duration::from_secs(5),
            true,
        );
        let note = observer.after_write(&victim).await;
        assert!(note.is_none());
        let on_disk = std::fs::read_to_string(&victim).unwrap();
        assert_eq!(
            on_disk, "hello world\n",
            "must never touch a path outside root"
        );
        std::fs::remove_dir_all(&project).ok();
        std::fs::remove_dir_all(&outside).ok();
    }
}