diff --git a/.config/jp/tools/src/fs.rs b/.config/jp/tools/src/fs.rs index 5106bb3c7..af8284315 100644 --- a/.config/jp/tools/src/fs.rs +++ b/.config/jp/tools/src/fs.rs @@ -12,7 +12,7 @@ mod list_files; mod modify_file; mod move_file; mod read_file; -mod utils; +pub(crate) mod utils; use create_file::fs_create_file; use delete_file::fs_delete_file; diff --git a/.config/jp/tools/src/lib.rs b/.config/jp/tools/src/lib.rs index 940aca547..bbf791685 100644 --- a/.config/jp/tools/src/lib.rs +++ b/.config/jp/tools/src/lib.rs @@ -5,6 +5,7 @@ mod debug_jp; mod fs; mod git; mod github; +mod markdown; mod plan; mod ticket; mod unix; @@ -26,6 +27,7 @@ pub async fn run(ctx: Context, t: Tool) -> util::ToolResult { s if s.starts_with("debug_jp_") => debug_jp::run(ctx, t).await, s if s.starts_with("web_") => web::run(ctx, t).await, s if s.starts_with("git_") => git::run(ctx, t).await, + s if s.starts_with("markdown_") => markdown::run(ctx, t), s if s.starts_with("unix_") => unix::run(ctx, t), s if s.starts_with("ticket_") => ticket::run(ctx, t), "plan" => plan::run(ctx, t), diff --git a/.config/jp/tools/src/markdown.rs b/.config/jp/tools/src/markdown.rs new file mode 100644 index 000000000..f3c83a453 --- /dev/null +++ b/.config/jp/tools/src/markdown.rs @@ -0,0 +1,136 @@ +//! The `markdown_format` tool: reflow standalone Markdown files with `comfort`. +//! +//! Covers the `.md` files that live outside Rust sources — RFDs, tickets, +//! READMEs, the docs site. +//! Doc comments inside `.rs` files are `cargo_format`'s territory. +//! +//! The `comfort` flags match the `fmt-markdown-ci` recipe in the justfile, so a +//! file this tool leaves alone is a file CI accepts. + +use std::collections::BTreeSet; + +use jp_tool::Context; + +use crate::{ + Tool, + fs::utils::resolve_workspace_path, + util::{ + OneOrMany, ToolResult, error, + runner::{DuctProcessRunner, ProcessOutput, ProcessRunner}, + truncate, unknown_tool, + }, +}; + +/// Cap for the formatted-file listing embedded in a tool result. +/// +/// A workspace-wide run after a broad edit can touch hundreds of files, and the +/// head of the list is what the reader needs. +const MAX_LISTING_BYTES: usize = 32_000; + +#[expect( + clippy::needless_pass_by_value, + reason = "consistent with other module run fns" +)] +pub fn run(ctx: Context, t: Tool) -> ToolResult { + match t.name.trim_start_matches("markdown_") { + "format" => markdown_format(&ctx, t.opt("paths")?), + _ => unknown_tool(t), + } +} + +fn markdown_format(ctx: &Context, paths: Option>) -> ToolResult { + markdown_format_impl(ctx, paths, &DuctProcessRunner) +} + +fn markdown_format_impl( + ctx: &Context, + paths: Option>, + runner: &R, +) -> ToolResult { + let selected = match select_paths(ctx, paths.unwrap_or_default().as_slice()) { + Ok(selected) => selected, + Err(message) => return error(message), + }; + + let mut args = vec![ + "--list-changed", + "--format-markdown", + "--reference-links", + "--prune-reference-links", + "--language", + "markdown", + ]; + if selected.is_empty() { + args.push("--workspace"); + } else { + args.extend(selected.iter().map(String::as_str)); + } + + let ProcessOutput { + stdout, + stderr, + status, + } = runner.run("comfort", &args, &ctx.root)?; + + if !status.is_success() { + return error(format!( + "comfort failed: {}", + truncate(&stderr, MAX_LISTING_BYTES) + )); + } + + // Workspace runs report absolute paths; strip the root so the listing reads + // the way the rest of the tools spell a path. + let mut files: BTreeSet = BTreeSet::new(); + for line in stdout.lines() { + let trimmed = line.trim(); + if !trimmed.is_empty() { + files.insert( + trimmed + .trim_start_matches(ctx.root.as_str()) + .trim_start_matches('/') + .to_owned(), + ); + } + } + + if files.is_empty() { + Ok("No files to format.".into()) + } else { + let listing = files.into_iter().collect::>().join("\n- "); + Ok(format!( + "Formatted files:\n- {}", + truncate(&listing, MAX_LISTING_BYTES) + ) + .into()) + } +} + +/// Validate each requested path and return its workspace-relative form. +/// +/// An empty request yields an empty selection, which the caller turns into a +/// whole-workspace run. +/// +/// `comfort` walks a directory and filters it down to Markdown extensions, but +/// takes a named file as-is whatever its extension — so a file that isn't +/// Markdown is refused here rather than rewritten as Markdown. +fn select_paths(ctx: &Context, paths: &[String]) -> Result, String> { + paths + .iter() + .map(|path| { + let resolved = resolve_workspace_path(&ctx.root, path, ctx.access.as_ref())?; + let markdown = matches!(resolved.relative.extension(), Some("md" | "markdown")); + if !markdown && !resolved.absolute.is_dir() { + return Err(format!( + "'{path}' is not a Markdown file or a directory. Only `.md` and `.markdown` \ + files are formatted." + )); + } + Ok(resolved.relative.into_string()) + }) + .collect() +} + +#[cfg(test)] +#[path = "markdown_tests.rs"] +mod tests; diff --git a/.config/jp/tools/src/markdown_tests.rs b/.config/jp/tools/src/markdown_tests.rs new file mode 100644 index 000000000..61b7d99ae --- /dev/null +++ b/.config/jp/tools/src/markdown_tests.rs @@ -0,0 +1,197 @@ +use std::fs; + +use camino_tempfile::{Utf8TempDir, tempdir}; +use jp_tool::{Action, Outcome}; +use pretty_assertions::assert_eq; + +use super::*; +use crate::util::runner::{ExitCode, MockProcessRunner, ProcessOutput}; + +/// The flags every invocation carries, in the order the tool builds them. +const BASE_ARGS: &[&str] = &[ + "--list-changed", + "--format-markdown", + "--reference-links", + "--prune-reference-links", + "--language", + "markdown", +]; + +fn ctx() -> (Utf8TempDir, Context) { + let dir = tempdir().unwrap(); + let ctx = Context { + root: dir.path().to_owned(), + action: Action::Run, + access: None, + workspace_id: "test".into(), + conversation_id: "test".into(), + }; + + (dir, ctx) +} + +fn paths(paths: &[&str]) -> OneOrMany { + OneOrMany::Many(paths.iter().map(|p| (*p).to_owned()).collect()) +} + +#[test] +fn no_paths_formats_the_whole_workspace() { + let (_dir, ctx) = ctx(); + let expected_args = [BASE_ARGS, &["--workspace"]].concat(); + let runner = MockProcessRunner::builder() + .expect("comfort") + .args(&expected_args) + .returns_success(""); + + let result = markdown_format_impl(&ctx, None, &runner).unwrap(); + assert_eq!(result.unwrap_content(), "No files to format."); +} + +#[test] +fn explicit_paths_replace_the_workspace_scope() { + let (_dir, ctx) = ctx(); + let expected_args = [BASE_ARGS, &["docs/rfd/drafts/D01-knowledge-base.md"]].concat(); + let runner = MockProcessRunner::builder() + .expect("comfort") + .args(&expected_args) + .returns_success(""); + + let result = markdown_format_impl( + &ctx, + Some(paths(&["docs/rfd/drafts/D01-knowledge-base.md"])), + &runner, + ) + .unwrap(); + assert_eq!(result.unwrap_content(), "No files to format."); +} + +#[test] +fn a_directory_is_passed_through_for_comfort_to_walk() { + let (dir, ctx) = ctx(); + fs::create_dir_all(dir.path().join("docs/rfd")).unwrap(); + let expected_args = [BASE_ARGS, &["docs/rfd"]].concat(); + let runner = MockProcessRunner::builder() + .expect("comfort") + .args(&expected_args) + .returns_success(""); + + let result = markdown_format_impl(&ctx, Some(paths(&["docs/rfd"])), &runner).unwrap(); + assert_eq!(result.unwrap_content(), "No files to format."); +} + +#[test] +fn changed_files_are_root_stripped_deduplicated_and_sorted() { + let (_dir, ctx) = ctx(); + let stdout = format!( + "{root}/docs/usage.md\n{root}/README.md\n{root}/docs/usage.md\n", + root = ctx.root + ); + let runner = MockProcessRunner::builder() + .expect("comfort") + .returns_success(stdout); + + let result = markdown_format_impl(&ctx, None, &runner).unwrap(); + assert_eq!( + result.unwrap_content(), + "Formatted files:\n- README.md\n- docs/usage.md" + ); +} + +#[test] +fn a_non_markdown_file_is_refused_without_running_comfort() { + let (_dir, ctx) = ctx(); + let runner = MockProcessRunner::never_called(); + + let result = + markdown_format_impl(&ctx, Some(paths(&["crates/jp_cli/src/main.rs"])), &runner).unwrap(); + match result { + Outcome::Error { message, .. } => assert_eq!( + message, + "'crates/jp_cli/src/main.rs' is not a Markdown file or a directory. Only `.md` and \ + `.markdown` files are formatted." + ), + _ => panic!("Expected Outcome::Error, got: {result:?}"), + } +} + +#[test] +fn a_path_outside_the_workspace_is_refused_without_running_comfort() { + let (_dir, ctx) = ctx(); + let runner = MockProcessRunner::never_called(); + + let result = + markdown_format_impl(&ctx, Some(paths(&["../elsewhere/notes.md"])), &runner).unwrap(); + match result { + Outcome::Error { message, .. } => { + assert_eq!(message, "Path must not escape the workspace root."); + } + _ => panic!("Expected Outcome::Error, got: {result:?}"), + } +} + +#[test] +fn one_bad_path_refuses_the_whole_request() { + let (_dir, ctx) = ctx(); + let runner = MockProcessRunner::never_called(); + + let result = markdown_format_impl( + &ctx, + Some(paths(&["docs/usage.md", "crates/jp_cli/src/main.rs"])), + &runner, + ) + .unwrap(); + match result { + Outcome::Error { message, .. } => assert!( + message.starts_with("'crates/jp_cli/src/main.rs' is not a Markdown file"), + "got: {message}" + ), + _ => panic!("Expected Outcome::Error, got: {result:?}"), + } +} + +#[test] +fn comfort_failure_is_reported() { + let (_dir, ctx) = ctx(); + let runner = MockProcessRunner::builder() + .expect("comfort") + .returns(ProcessOutput { + stdout: String::new(), + stderr: "comfort: parse error".to_owned(), + status: ExitCode::from_code(2), + }); + + let result = markdown_format_impl(&ctx, None, &runner).unwrap(); + match result { + Outcome::Error { message, .. } => { + assert_eq!(message, "comfort failed: comfort: parse error"); + } + _ => panic!("Expected Outcome::Error, got: {result:?}"), + } +} + +#[test] +fn formatted_file_listing_is_bounded() { + let (_dir, ctx) = ctx(); + let stdout = (0..4_000) + .map(|i| format!("{root}/docs/generated/file_{i}.md", root = ctx.root)) + .collect::>() + .join("\n"); + let runner = MockProcessRunner::builder() + .expect("comfort") + .returns_success(stdout); + + let content = markdown_format_impl(&ctx, None, &runner) + .unwrap() + .unwrap_content(); + + assert!( + content.len() < MAX_LISTING_BYTES + 200, + "listing grew to {} bytes", + content.len() + ); + assert!( + content.contains("[Truncated: showing"), + "got tail: {}", + &content[content.len() - 100..] + ); +} diff --git a/.jp/config/skill/rfd.toml b/.jp/config/skill/rfd.toml index a511c552b..0396d2111 100644 --- a/.jp/config/skill/rfd.toml +++ b/.jp/config/skill/rfd.toml @@ -9,6 +9,8 @@ attachments = [ ] [conversation.tools] +markdown_format = { enable = true } + rfd_draft = { enable = true, run = "unattended" } rfd_promote = { enable = true, run = "unattended" } rfd_renumber = { enable = true, run = "unattended" } @@ -117,8 +119,8 @@ hedging language and filler. an RFD, use `fs_modify_file` to make targeted edits. The user will be \ asked to confirm each modification before it is applied. -8. **Format before hashing**: Run `cargo_format` before computing an \ -RFD's hash for `docs/.vitepress/rfd-summaries.json` — `comfort` reflows \ +8. **Format before hashing**: Run `markdown_format` before computing an \ +RFD's hash for `docs/.vitepress/rfd-summaries.json` — it reflows \ markdown paragraphs, so a hash taken first goes stale.\ """ diff --git a/.jp/config/skill/writing.toml b/.jp/config/skill/writing.toml index 633cbfc76..720a7f783 100644 --- a/.jp/config/skill/writing.toml +++ b/.jp/config/skill/writing.toml @@ -11,8 +11,14 @@ You have been given the technical-writing skill. You produce clear, scannable, \ developer-facing documentation for command-line developer tools. You combine \ the read-files skill with documentation-research methodology to verify claims \ against the actual code, tests, and existing docs before writing about them. + +`markdown_format` reflows the Markdown files you write and canonicalizes their \ +structure and reference links. Run it after editing a `.md` file. """ +[conversation.tools] +markdown_format = { enable = true } + [[assistant.instructions]] title = "Documentation Research" items = [ diff --git a/.jp/mcp/tools/markdown/format.toml b/.jp/mcp/tools/markdown/format.toml new file mode 100644 index 000000000..27a8ac763 --- /dev/null +++ b/.jp/mcp/tools/markdown/format.toml @@ -0,0 +1,41 @@ +[conversation.tools.markdown_format] +enable = false +run = "unattended" + +source = "local" +command = "just serve-tools {{context}} {{tool}}" +summary = "Format standalone Markdown files: reflow paragraphs, canonicalize structure, and normalize reference links." +description = """ +Runs `comfort` over Markdown files with the same flags CI enforces, so a \ +formatted file is one CI accepts. + +This covers `.md` files only — RFDs, tickets, READMEs, the docs site. Markdown \ +inside Rust doc comments is `cargo_format`'s job. +""" + +examples = """ +Format every Markdown file in the workspace: +```json +{} +``` + +Format a single draft: +```json +{"paths": ["docs/rfd/drafts/D01-knowledge-base.md"]} +``` + +Format a directory tree: +```json +{"paths": ["docs/rfd", "docs/ticket"]} +``` +""" + +[conversation.tools.markdown_format.style] +inline_results = "off" +results_file_link = "off" +parameters = "function_call" + +[conversation.tools.markdown_format.parameters.paths] +type = "array" +items.type = "string" +summary = "Markdown files or directories to format. Directories are walked recursively. If omitted, every Markdown file in the workspace is formatted."