diff --git a/.config/jp/tools/src/git.rs b/.config/jp/tools/src/git.rs index 4cabeb37..6a8dce83 100644 --- a/.config/jp/tools/src/git.rs +++ b/.config/jp/tools/src/git.rs @@ -19,6 +19,7 @@ mod log; mod show; mod stage_patch; mod stage_patch_lines; +mod status; mod unstage; use add_intent::git_add_intent; @@ -32,6 +33,7 @@ use log::git_log; use show::git_show; use stage_patch::git_stage_patch; use stage_patch_lines::git_stage_patch_lines; +use status::git_status; use unstage::git_unstage; pub async fn run(ctx: Context, t: Tool) -> ToolResult { @@ -71,6 +73,8 @@ pub async fn run(ctx: Context, t: Tool) -> ToolResult { "show" => git_show(ctx.root, t.req("revision")?, opts).await, + "status" => git_status(ctx.root, opts).await, + "blame" => { git_blame( ctx.root, diff --git a/.config/jp/tools/src/git/status.rs b/.config/jp/tools/src/git/status.rs new file mode 100644 index 00000000..52bea1e8 --- /dev/null +++ b/.config/jp/tools/src/git/status.rs @@ -0,0 +1,163 @@ +use std::fmt::Write; + +use camino::{Utf8Path, Utf8PathBuf}; +use serde_json::{Map, Value}; + +use super::env_from_options; +use crate::util::{ + ToolResult, error, + runner::{DuctProcessRunner, ProcessRunner}, +}; + +/// Maximum number of untracked files to list before truncating. +/// +/// Tracked changes are always shown in full; only the untracked listing is +/// capped, so a large untracked directory (e.g. non-ignored build output) can't +/// bury the tracked edits this guard exists to surface. +const MAX_UNTRACKED: usize = 500; + +/// A single entry from `git status --porcelain`. +#[derive(Debug, PartialEq)] +struct StatusEntry { + /// The two-character XY status code (e.g. + /// `" M"`, `"??"`, `"A "`). + code: String, + /// The path, or `old -> new` for renames and copies. + path: String, +} + +pub(crate) async fn git_status(root: Utf8PathBuf, options: &Map) -> ToolResult { + let env = env_from_options(options); + git_status_impl(&root, &DuctProcessRunner, &env) +} + +fn git_status_impl( + root: &Utf8Path, + runner: &R, + env: &[(&str, &str)], +) -> ToolResult { + // `core.quotePath=false` keeps non-ASCII paths readable instead of + // octal-escaped. `--untracked-files=all` expands untracked directories to + // individual files (the default collapses them to `dir/`), so the guard + // reports the actual paths an assistant needs to rule out local edits. + let output = runner.run_with_env( + "git", + &[ + "-c", + "core.quotePath=false", + "status", + "--porcelain", + "--untracked-files=all", + ], + root, + env, + )?; + + if !output.status.is_success() { + return error(format!("git status failed: {}", output.stderr.trim())); + } + + let entries = parse_status(&output.stdout); + Ok(format_status(&entries).into()) +} + +/// Parse `git status --porcelain` v1 output into per-file entries. +/// +/// Each line is `XYPATH`, where `XY` is the two-character status code +/// and `PATH` is `old -> new` for renames and copies. +fn parse_status(stdout: &str) -> Vec { + stdout + .lines() + .filter_map(|line| { + // Shortest valid line is `XY P` (code, space, one path char). + if line.len() < 4 { + return None; + } + let (code, rest) = line.split_at(2); + Some(StatusEntry { + code: code.to_string(), + // Strip exactly the one separator space after the XY code; a + // path that itself begins with spaces must survive intact. + path: rest.strip_prefix(' ').unwrap_or(rest).to_string(), + }) + }) + .collect() +} + +fn format_status(entries: &[StatusEntry]) -> String { + if entries.is_empty() { + return "Working tree clean.".to_string(); + } + + let (untracked, tracked): (Vec<&StatusEntry>, Vec<&StatusEntry>) = + entries.iter().partition(|e| e.code == "??"); + + let mut out = String::from("\n"); + + for e in tracked { + let _ = writeln!(out, " - {} ({})", e.path, describe(&e.code)); + } + + for e in untracked.iter().take(MAX_UNTRACKED) { + let _ = writeln!(out, " - {} ({})", e.path, describe(&e.code)); + } + + if untracked.len() > MAX_UNTRACKED { + let _ = writeln!( + out, + " ... and {} more untracked files not shown (output truncated).", + untracked.len() - MAX_UNTRACKED + ); + } + + out.push_str(""); + out +} + +/// Decode a porcelain XY status code into a human-readable description. +/// +/// The index (staged) and worktree (unstaged) columns are reported separately, +/// so `MM` becomes "modified, staged; modified, unstaged". +fn describe(code: &str) -> String { + if code == "??" { + return "untracked".to_string(); + } + if code == "!!" { + return "ignored".to_string(); + } + + let mut chars = code.chars(); + let index = chars.next().unwrap_or(' '); + let worktree = chars.next().unwrap_or(' '); + + let mut parts = Vec::new(); + if let Some(word) = describe_char(index) { + parts.push(format!("{word}, staged")); + } + if let Some(word) = describe_char(worktree) { + parts.push(format!("{word}, unstaged")); + } + + if parts.is_empty() { + code.trim().to_string() + } else { + parts.join("; ") + } +} + +fn describe_char(c: char) -> Option<&'static str> { + match c { + 'M' => Some("modified"), + 'A' => Some("added"), + 'D' => Some("deleted"), + 'R' => Some("renamed"), + 'C' => Some("copied"), + 'U' => Some("unmerged"), + 'T' => Some("type changed"), + _ => None, + } +} + +#[cfg(test)] +#[path = "status_tests.rs"] +mod tests; diff --git a/.config/jp/tools/src/git/status_tests.rs b/.config/jp/tools/src/git/status_tests.rs new file mode 100644 index 00000000..72359eee --- /dev/null +++ b/.config/jp/tools/src/git/status_tests.rs @@ -0,0 +1,137 @@ +use camino_tempfile::tempdir; + +use super::*; +use crate::util::runner::MockProcessRunner; + +#[test] +fn parses_porcelain_entries() { + let stdout = " M src/foo.rs\n?? new.txt\nA staged.rs\nR old.rs -> new.rs\n"; + let entries = parse_status(stdout); + + assert_eq!(entries.len(), 4); + assert_eq!(entries[0], StatusEntry { + code: " M".into(), + path: "src/foo.rs".into(), + }); + assert_eq!(entries[1], StatusEntry { + code: "??".into(), + path: "new.txt".into(), + }); + assert_eq!(entries[2], StatusEntry { + code: "A ".into(), + path: "staged.rs".into(), + }); + assert_eq!(entries[3], StatusEntry { + code: "R ".into(), + path: "old.rs -> new.rs".into(), + }); +} + +#[test] +fn keeps_leading_space_in_path() { + // Porcelain is a fixed `XYPATH`; only the one separator after the + // status code is stripped, so a path that itself begins with spaces must + // survive intact (the dirty-tree check relies on the real path). + let entries = parse_status("?? leading.rs\n"); + + assert_eq!(entries.len(), 1); + assert_eq!(entries[0], StatusEntry { + code: "??".into(), + path: " leading.rs".into(), + }); +} + +#[test] +fn describes_codes() { + assert_eq!(describe("??"), "untracked"); + assert_eq!(describe(" M"), "modified, unstaged"); + assert_eq!(describe("M "), "modified, staged"); + assert_eq!(describe("MM"), "modified, staged; modified, unstaged"); + assert_eq!(describe("A "), "added, staged"); + assert_eq!(describe("D "), "deleted, staged"); + assert_eq!(describe("R "), "renamed, staged"); +} + +#[test] +fn formats_clean_tree() { + assert_eq!(format_status(&[]), "Working tree clean."); +} + +#[test] +fn basic_status() { + let dir = tempdir().unwrap(); + let runner = MockProcessRunner::success(" M src/foo.rs\n?? new.txt\n"); + + let content = git_status_impl(dir.path(), &runner, &[]) + .unwrap() + .into_content() + .unwrap(); + + assert!(content.contains("- src/foo.rs (modified, unstaged)")); + assert!(content.contains("- new.txt (untracked)")); +} + +#[test] +fn requests_all_untracked_files() { + let dir = tempdir().unwrap(); + let runner = MockProcessRunner::builder() + .expect("git") + .args(&[ + "-c", + "core.quotePath=false", + "status", + "--porcelain", + "--untracked-files=all", + ]) + .returns_success(""); + + let _outcome = git_status_impl(dir.path(), &runner, &[]).unwrap(); +} + +#[test] +fn caps_untracked_but_always_shows_tracked() { + // A large untracked directory must not bury the tracked edits the guard + // exists to surface: tracked changes are shown in full, untracked are + // capped with a count of the remainder. + let mut entries = vec![StatusEntry { + code: " M".into(), + path: "src/keep.rs".into(), + }]; + for i in 0..(MAX_UNTRACKED + 50) { + entries.push(StatusEntry { + code: "??".into(), + path: format!("junk/{i}.tmp"), + }); + } + + let out = format_status(&entries); + + assert!( + out.contains("- src/keep.rs (modified, unstaged)"), + "tracked change must always show" + ); + assert!(out.contains("and 50 more untracked files not shown")); + assert_eq!(out.matches("(untracked)").count(), MAX_UNTRACKED); +} + +#[test] +fn clean_tree_via_runner() { + let dir = tempdir().unwrap(); + let runner = MockProcessRunner::success(""); + + let content = git_status_impl(dir.path(), &runner, &[]) + .unwrap() + .into_content() + .unwrap(); + + assert_eq!(content, "Working tree clean."); +} + +#[test] +fn status_git_error() { + let dir = tempdir().unwrap(); + let runner = MockProcessRunner::error("fatal: not a git repository"); + + let outcome = git_status_impl(dir.path(), &runner, &[]).unwrap(); + assert!(outcome.into_content().is_none(), "expected error outcome"); +} diff --git a/.jp/config/personas/pr-reviewer.toml b/.jp/config/personas/pr-reviewer.toml index de6f4009..28cf6b63 100644 --- a/.jp/config/personas/pr-reviewer.toml +++ b/.jp/config/personas/pr-reviewer.toml @@ -69,11 +69,15 @@ items = [ with a reply, or leave them alone.\ """, """\ - **2. Read for context, not just the diff.** When a hunk is too narrow to judge, use \ - `github_read_file` (with `start_line` / `end_line` for any non-trivial file) to fetch the \ - surrounding code at the PR's branch, or `github_pr_commits` / `github_commit` to inspect \ - the PR's individual commits. (The local `git_log` / `git_diff_commit` tools only see \ - commits already on your checked-out branch, not the PR's.)\ + **2. Read for context, not just the diff.** When a hunk is too narrow to judge, read the \ + surrounding code. If the prompt says the working tree is checked out at the PR's head, \ + prefer the local `fs_*` and `git_*` tools — they are faster and complete (if it also says \ + the tree is dirty, call `git_status` to list the changed files and confirm they don't affect \ + your read). \ + Otherwise the working tree does not match the PR, so use `github_read_file` (with \ + `start_line` / `end_line`) to read code at the PR's branch and `github_pr_commits` / \ + `github_commit` for its commits; the local `git_log` / `git_diff_commit` tools only see \ + commits already on your checked-out branch.\ """, """\ **3. Cross-reference.** Use `github_issues` and `github_code_search` to find related work \ diff --git a/.jp/config/personas/pr-triager.toml b/.jp/config/personas/pr-triager.toml index 40f5e7c6..0e17c881 100644 --- a/.jp/config/personas/pr-triager.toml +++ b/.jp/config/personas/pr-triager.toml @@ -77,11 +77,15 @@ items = [ """, """\ **3. Ground each item against evidence.** Use `fs_grep_files`, `fs_read_file`, and \ - `fs_list_files` to inspect the actual code at the lines under discussion. Use `git_log`, \ - `git_show`, and `git_diff_commit` for history on your local branch, and `github_pr_commits` \ - / `github_commit` to inspect the PR's own commits when they aren't checked out locally. Use \ - `cargo_check` and `cargo_test` if a comment claims a behavior you can verify by compiling or \ - running tests. Use `github_issues` and `github_code_search` for context outside this PR.\ + `fs_list_files` to inspect the actual code at the lines under discussion. If the prompt says \ + the working tree is checked out at the PR's head, prefer the local `git_log`, `git_show`, \ + and `git_diff_commit` tools for history — faster and complete (if it also says the tree is \ + dirty, call `git_status` to list the changed files and confirm they don't affect your read). \ + Otherwise the \ + working tree does not match the PR, so use `github_pr_commits` / `github_commit` to inspect \ + its commits. Use `cargo_check` and `cargo_test` if a comment claims a behavior you can \ + verify by compiling or running tests. Use `github_issues` and `github_code_search` for \ + context outside this PR.\ """, """\ **4. Think hard** before writing. The value of this turn is in the quality of the \ diff --git a/.jp/config/skill/git-reading.toml b/.jp/config/skill/git-reading.toml index 07b533c7..aab60df1 100644 --- a/.jp/config/skill/git-reading.toml +++ b/.jp/config/skill/git-reading.toml @@ -12,6 +12,8 @@ use `git_diff_file` for that. - git_diff_file: Working-tree analog of `git_diff_commit`. Show the full staged or unstaged \ diff for explicitly listed files. Requires `status` and `paths`. Supports `pattern` to grep \ within large diffs and `start_line` / `end_line` to page through large diffs in chunks. +- git_status: Show the working tree status — which files are modified, staged, or untracked \ +(untracked files do not appear in `git_diff`). - git_log: Search the commit history. Supports filtering by message text, file paths, date range, \ and result count. - git_show: Show commit details (message and changed file stats). Does NOT include actual diff \ @@ -28,6 +30,7 @@ Use these tools in a drill-down workflow: `git_diff_commit` to view the actual diff for specific files. - For the working tree: `git_diff` for the overview, `git_diff_file` to drill into a file \ whose diff was truncated. +- For working-tree state: `git_status` to list modified / staged / untracked files. - For line history: `git_blame` to find which commit introduced a line, then `git_show` or \ `git_diff_commit` on the blamed (or `previous`) hash to see why. """ @@ -35,6 +38,7 @@ whose diff was truncated. [conversation.tools] git_diff = { enable = true, run = "unattended", result = "unattended", style.inline_results = "off", style.results_file_link = "off" } git_diff_file = { enable = true, run = "unattended", result = "unattended", style.inline_results = "full", style.results_file_link = "off" } +git_status = { enable = true, run = "unattended", result = "unattended", style.inline_results = "full", style.results_file_link = "off" } git_log = { enable = true, run = "unattended", result = "unattended", style.inline_results = "full", style.results_file_link = "off" } git_show = { enable = true, run = "unattended", result = "unattended", style.inline_results = "full", style.results_file_link = "off" } git_diff_commit = { enable = true, run = "unattended", result = "unattended", style.inline_results = "full", style.results_file_link = "off" } diff --git a/.jp/mcp/tools/git/status.toml b/.jp/mcp/tools/git/status.toml new file mode 100644 index 00000000..ba3dc7de --- /dev/null +++ b/.jp/mcp/tools/git/status.toml @@ -0,0 +1,23 @@ +[conversation.tools.git_status] +enable = "explicit" +run = "unattended" +style.inline_results = "full" +style.parameters = "function_call" + +source = "local" +command = "just serve-tools {{context}} {{tool}}" +summary = "Show the working tree status: modified, staged, and untracked files." +description = """ +Lists every path with uncommitted changes in the working tree, including \ +untracked files (which `git_diff` does not show). Untracked directories are \ +expanded to individual files; a very large untracked listing is truncated. \ +Reports, per file, whether the change is staged, unstaged, or the file is \ +untracked. Use `git_diff` or `git_diff_file` to see the actual content changes. +""" + +examples = """ +Show the current working tree status: +```json +{} +``` +""" diff --git a/justfile b/justfile index 758aa3d0..c4328711 100644 --- a/justfile +++ b/justfile @@ -144,7 +144,7 @@ rfd-this *ARGS: _install-jp # submit it from the GitHub UI. [group('jp')] [positional-arguments] -pr-review NNN *ARGS: _install-jp +pr-review NNN *ARGS: _install-jp _install-tools #!/usr/bin/env sh set -eu @@ -165,6 +165,20 @@ pr-review NNN *ARGS: _install-jp args=$(just _shape-args "$msg" "$@") + # Tell the reviewer whether the working tree holds this PR's code, so it + # prefers the local fs_*/git_* tools over the slower github_* ones. + state=$(just _pr-checkout-state {{NNN}}) + case "$state" in + "LOCAL "*) args="$args\n\nThe working tree is checked out at this PR's head \ + (${state#LOCAL }) and clean. Prefer the local fs_* and git_* tools over github_* for \ + reading files and history; they are faster and complete." ;; + "DIRTY "*) args="$args\n\nThe working tree is checked out at this PR's head \ + (${state#DIRTY }) but has uncommitted local modifications. You may use the local fs_* \ + and git_* tools; first call git_status to see which files differ (including untracked), \ + then confirm none of those changes affect what you're reviewing before trusting a local \ + read." ;; + esac + title="pr-review:{{NNN}}" existing="" @@ -207,7 +221,7 @@ pr-review NNN *ARGS: _install-jp # useful when the conversation has gone off the rails. [group('jp')] [positional-arguments] -pr-triage NNN *ARGS: _install-jp +pr-triage NNN *ARGS: _install-jp _install-tools #!/usr/bin/env sh set -eu @@ -229,6 +243,46 @@ pr-triage NNN *ARGS: _install-jp args=$(just _shape-args "$msg" "$@") + # Tell the triager whether the working tree holds this PR's code, so it + # prefers the local fs_*/git_* tools over the slower github_* ones. + state=$(just _pr-checkout-state {{NNN}}) + case "$state" in + "LOCAL "*) args="$args\n\nThe working tree is checked out at this PR's head \ + (${state#LOCAL }) and clean. Prefer the local fs_* and git_* tools over github_* for \ + reading files and history; they are faster and complete." ;; + "DIRTY "*) args="$args\n\nThe working tree is checked out at this PR's head \ + (${state#DIRTY }) but has uncommitted local modifications. You may use the local fs_* \ + and git_* tools; first call git_status to see which files differ (including untracked), \ + then confirm none of those changes affect what you're triaging before trusting a local \ + read." ;; + esac + + # On the PR's branch, the implementation conversation is probably in this + # session. Offer to triage there — picking from session-bound conversations + # — instead of a fresh, context-free one. `jp query` with no target then + # runs in whatever conversation the picker activates. + case "$state" in + "LOCAL "*|"DIRTY "*) + if [ -r /dev/tty ] && [ -w /dev/tty ]; then + printf "You're on PR #{{NNN}}'s branch.\n" > /dev/tty + printf " Triage in a [p]icked conversation / [n]ew triage conversation / [q]uit: " > /dev/tty + IFS= read -r ans < /dev/tty + else + ans=n + fi + case "$ans" in + p|P) + jp conversation use '?session' + jp query --cfg=personas/pr-triager \ + --attach "gh:pull/{{NNN}}/diff" \ + --attach "gh:pull/{{NNN}}/reviews?include_outdated=true" \ + $args + exit 0 ;; + q|Q) exit 0 ;; + *) ;; + esac ;; + esac + title="pr-triage:{{NNN}}" existing="" @@ -1864,6 +1918,18 @@ _install-jp *args: fi cargo install {{quiet_flag}} --locked --path crates/jp_cli {{args}} +# Build and install the `jp-tools` binary that `serve-tools` runs for local MCP +# tools. Recipes that invoke local tools (e.g. `git_status`) depend on this so a +# stale binary doesn't return `Unknown tool` for a tool added in the checkout. +_install-tools *args: + #!/usr/bin/env sh + set -eu + if [ -n "${JP_NO_INSTALL:-}" ]; then + echo "Skipping jp-tools rebuild (JP_NO_INSTALL set); using the installed binary." >&2 + exit 0 + fi + cargo install {{quiet_flag}} --locked --path .config/jp/tools --debug {{args}} + @_install-comfort *args: cargo install {{quiet_flag}} --locked --path crates/contrib/comfort {{args}} @@ -1924,6 +1990,39 @@ _resolve-conversation TITLE: *) echo "Unknown choice '$choice'; aborting." >&2; exit 1 ;; esac +# Internal: report whether the working tree holds the given PR's code. +# +# Resolves the PR head sha from refs/pull/N/head on the dcdpr/jp remote (no gh +# needed) and compares it to local HEAD. Outputs one of: +# +# LOCAL - HEAD is the PR head and the tree is clean +# DIRTY - HEAD is the PR head but the tree has uncommitted changes +# REMOTE - tree doesn't match the PR, or the head can't be resolved +# +# Callers inject the result into the prompt so the assistant knows whether to +# prefer the local fs_*/git_* tools over the slower github_* ones. +[no-exit-message] +[private] +_pr-checkout-state NNN: + #!/usr/bin/env sh + set -eu + + remote=$(git remote -v 2>/dev/null | awk '/dcdpr\/jp/ {print $1; exit}') + remote=${remote:-origin} + + head_sha=$(git ls-remote "$remote" "refs/pull/{{NNN}}/head" 2>/dev/null \ + | awk '{print $1; exit}') + + [ -n "$head_sha" ] || { echo "REMOTE"; exit 0; } + [ "$head_sha" = "$(git rev-parse HEAD 2>/dev/null || true)" ] \ + || { echo "REMOTE"; exit 0; } + + if [ -n "$(git status --porcelain 2>/dev/null)" ]; then + echo "DIRTY $head_sha" + else + echo "LOCAL $head_sha" + fi + # Internal: look up a Bear note (or notes) by tag. # # Resolves `bear://search/?tag=TAG` against the local Bear database. Archived