From dbf51f3329a4dc70f1dbb74330a1519bece85194 Mon Sep 17 00:00:00 2001 From: Prakhar Khatri Date: Mon, 27 Apr 2026 10:39:55 +0000 Subject: [PATCH] feat: add AgentDiff context workflow Preserve structured context in traces, surface it in reports and file-scoped context, and add a Cursor skill installer so agents can use the workflow consistently. --- .cursor/skills/agentdiff-context/SKILL.md | 74 ++++ Cargo.lock | 2 +- Cargo.toml | 2 +- docs/context-workflow.md | 110 +++++ scripts/finalize-ledger.py | 8 + scripts/record-context.py | 15 +- scripts/tests/test_capture_prompts.py | 47 ++ scripts/tests/test_record_context.py | 47 ++ src/cli.rs | 48 ++- src/commands/context.rs | 149 +++++++ src/commands/install_skill.rs | 88 ++++ src/commands/mod.rs | 4 +- src/commands/report.rs | 495 +++++++++++++++++++++- src/main.rs | 8 +- src/store.rs | 140 ++++-- 15 files changed, 1175 insertions(+), 62 deletions(-) create mode 100644 .cursor/skills/agentdiff-context/SKILL.md create mode 100644 docs/context-workflow.md create mode 100644 scripts/tests/test_record_context.py create mode 100644 src/commands/context.rs create mode 100644 src/commands/install_skill.rs diff --git a/.cursor/skills/agentdiff-context/SKILL.md b/.cursor/skills/agentdiff-context/SKILL.md new file mode 100644 index 0000000..ef9607f --- /dev/null +++ b/.cursor/skills/agentdiff-context/SKILL.md @@ -0,0 +1,74 @@ +--- +name: agentdiff-context +description: Use AgentDiff trace context during development and review. Use when editing traced files, preparing PR summaries, reviewing agent-authored code, or when the user asks about why a file changed, attribution, intent, files read, or AgentDiff context. +--- + +# AgentDiff Context + +## When To Use + +Use this skill when working in a repository that has AgentDiff initialized and the task involves: + +- editing an existing file that may have AI attribution +- reviewing a PR or writing a PR summary +- explaining why a file or line range changed +- recording agent intent before a commit +- deciding which files deserve review attention first + +## Workflow + +1. Before editing a known file, check its trace context: + +```bash +agentdiff context path/to/file --json +``` + +2. For PR review or summary work, inspect structured report context: + +```bash +agentdiff report --format json --context +agentdiff report --format markdown --context +``` + +3. Use the context to answer: + +- What intent produced this change? +- Which agent/model touched it? +- Which files were read to make the change? +- Which trace IDs explain the relevant file/ranges? +- Are there flags like `security`, `refactor`, or `risky`? + +4. Before committing substantial agent work, record context through AgentDiff MCP when available. Include concise values: + +```json +{ + "prompt": "short user request or task summary", + "model_id": "model name", + "agent": "cursor", + "files_read": ["src/api.rs", "src/config.rs"], + "intent": "security hardening", + "trust": 80, + "flags": ["security"] +} +``` + +## Rules + +- Do not paste full prompts into commit messages or PR summaries. Use AgentDiff's stored prompt excerpt/hash. +- Keep `intent` short and review-useful, for example `security hardening`, `bug fix`, `test coverage`, or `refactor`. +- If `agentdiff context` returns no traces, say so and continue with normal code reading. +- Do not treat AgentDiff attribution as proof of correctness. It is context for review, not validation. +- If context conflicts with code reality, trust the code and mention the mismatch. + +## Output Guidance + +When summarizing AgentDiff context to a user, prefer this shape: + +```markdown +AgentDiff context for `path/to/file`: +- Last traced intent: ... +- Agent/model: ... +- Relevant ranges: ... +- Files read: ... +- Review note: ... +``` diff --git a/Cargo.lock b/Cargo.lock index 90186a0..28fb9a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,7 +4,7 @@ version = 4 [[package]] name = "agentdiff" -version = "0.1.25" +version = "0.1.26" dependencies = [ "anyhow", "assert_cmd", diff --git a/Cargo.toml b/Cargo.toml index 7727009..9c2cfe4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "agentdiff" -version = "0.1.25" +version = "0.1.26" edition = "2024" rust-version = "1.85" description = "Audit and trace autonomous AI code contributions in git repositories" diff --git a/docs/context-workflow.md b/docs/context-workflow.md new file mode 100644 index 0000000..a906703 --- /dev/null +++ b/docs/context-workflow.md @@ -0,0 +1,110 @@ +# AgentDiff Context Workflow + +This note records the context work added to AgentDiff so the implementation and validation criteria do not live only in chat history. + +## What Changed + +AgentDiff now preserves more of the structured context agents already provide before a commit. `scripts/finalize-ledger.py` writes these fields into `metadata.agentdiff` on each `AgentTrace`: + +- `intent` +- `files_read` +- `author` +- `capture_tool` +- existing prompt excerpt/hash, session id, trust, and flags + +Prompt text still follows the `capture_prompts` privacy gate. Intent and files-read are stored because they are explicit structured context, not raw transcript capture. + +## Report Context + +`agentdiff report --format markdown --context` adds a compact review section for humans and PR bots: + +- a per-agent line summary +- grouped review context by intent +- files read, flags, trust, and prompt excerpt when available +- a `Files To Review First` table with trace ids +- collapsed trace details for deeper inspection + +`agentdiff report --format json --context` exposes the same trace metadata for bots or custom tooling. + +The goal is not to make the PR comment long. The goal is to help a reviewer quickly decide which files deserve attention first and why the agent changed them. + +When posting a PR comment with `--post-pr-comment`, AgentDiff filters the report to commits on the current branch since the merge-base with the default branch. This keeps old consolidated traces from `main` out of the PR review comment. Comment posting first tries to edit the last AgentDiff comment and falls back to creating one, so it works with older `gh` versions that do not support `--create-if-none`. + +## Local File Context + +`agentdiff context ` shows trace context for one file: + +```bash +agentdiff context src/api.rs +agentdiff context src/api.rs --json +``` + +This is the local agent-facing surface. Before editing a traced file, an agent can inspect: + +- who or what last changed the file +- the recorded intent +- relevant line ranges +- files read when the change was made +- flags and trust metadata + +This should help agents avoid losing project context in large codebases, but it is not a replacement for reading the code. + +## Cursor Skill + +The project skill at `.cursor/skills/agentdiff-context/SKILL.md` tells agents to: + +- run `agentdiff context --json` before modifying traced files +- run `agentdiff report --format json --context` before PR review or summaries +- record concise intent and files-read context before substantial commits + +The skill helps local Cursor agents. Generic GitHub review bots only benefit if they read the PR comment, consume the JSON report, or run AgentDiff themselves. + +Install it into a repository with: + +```bash +agentdiff install-skill --scope project +``` + +Global install is available for personal defaults, but project install is preferred because repository-specific AgentDiff guidance should be versioned with the code: + +```bash +agentdiff install-skill --scope global +``` + +## Validation In `~/roam/monorepo` + +The realistic validation target is the local `~/roam/monorepo` checkout for GitHub repo `roam-agentdiff`. + +Validation should check: + +- `agentdiff push` is fast with no new traces and with one new trace +- `agentdiff report --format markdown --context` stays readable +- `agentdiff report --format json --context` is parseable +- `agentdiff context --json` returns useful intent/files-read metadata +- the CI PR comment is updated rather than duplicated + +The release validation PR was opened at `https://github.com/codeprakhar25/roam-agentdiff/pull/6`. It installed the AgentDiff context skill and added a small validation note. The PR comment was posted with a local AgentDiff build and correctly filtered out old `main` traces. + +Measured timings in `~/roam/monorepo`: + +- `agentdiff push` with 2 new traces: `5.09s` +- `agentdiff push` with no local traces: `0.00s` +- `agentdiff report --format markdown --context`: `0.02s` +- `agentdiff report --format json --context`: `0.02s` +- `agentdiff context agentdiff-context-validation.md --json`: `0.01s` + +The validation trace included: + +- intent: `agentdiff context validation` +- files read: `agentdiff-context-smoke.txt`, `README.md` +- flags: `validation` +- trust: `90` + +The final PR validation used intent `agentdiff release validation` with files read `agentdiff-context-validation.md` and `README.md`. The generated PR comment surfaced that intent in `Review Context`, and file-scoped JSON returned the same metadata for the changed file. + +## Known Limitations + +- AgentDiff context is review context, not correctness evidence. +- Large PR comments still need size discipline; detailed trace data should stay collapsed or in JSON. +- Older traces may not include intent/files-read, so reports must handle `unspecified`. +- Bots that do not read AgentDiff output will not benefit from the skill alone. diff --git a/scripts/finalize-ledger.py b/scripts/finalize-ledger.py index 7168f32..a0dd326 100644 --- a/scripts/finalize-ledger.py +++ b/scripts/finalize-ledger.py @@ -156,6 +156,14 @@ def write_agent_trace(repo_root: str, pending: dict, sha: str, ts: str) -> Optio metadata["flags"] = pending["flags"] if pending.get("session_id"): metadata["session_id"] = str(pending["session_id"]) + if pending.get("intent"): + metadata["intent"] = str(pending["intent"]) + if isinstance(pending.get("files_read"), list) and pending["files_read"]: + metadata["files_read"] = [str(p) for p in pending["files_read"]] + if git_author: + metadata["author"] = git_author + if pending.get("tool"): + metadata["capture_tool"] = str(pending["tool"]) trace: dict = { "version": "0.1.0", diff --git a/scripts/record-context.py b/scripts/record-context.py index 766e31a..d3e6ca8 100644 --- a/scripts/record-context.py +++ b/scripts/record-context.py @@ -12,6 +12,7 @@ import argparse import json import os +import select import subprocess import sys from datetime import datetime, timezone @@ -42,6 +43,18 @@ def parse_json_array(raw: str): return [] +def read_available_stdin() -> str: + if sys.stdin.isatty(): + return "" + try: + ready, _, _ = select.select([sys.stdin], [], [], 0) + except Exception: + return "" + if not ready: + return "" + return sys.stdin.read() + + def main() -> int: parser = argparse.ArgumentParser(add_help=True) parser.add_argument("--cwd", default=os.getcwd()) @@ -56,7 +69,7 @@ def main() -> int: args = parser.parse_args() payload = {} - stdin = sys.stdin.read() + stdin = read_available_stdin() if stdin.strip(): try: obj = json.loads(stdin) diff --git a/scripts/tests/test_capture_prompts.py b/scripts/tests/test_capture_prompts.py index 243782d..a001b66 100644 --- a/scripts/tests/test_capture_prompts.py +++ b/scripts/tests/test_capture_prompts.py @@ -4,7 +4,9 @@ Covers both capture-claude.py (capture side) and finalize-ledger.py (trace side). """ import importlib.util +import json import os +import subprocess import tempfile import unittest from pathlib import Path @@ -161,6 +163,51 @@ def test_returns_true_when_enabled(self): if original is not None: os.environ["HOME"] = original + def test_write_agent_trace_persists_structured_context_metadata(self): + with tempfile.TemporaryDirectory() as tmp: + repo = Path(tmp) / "repo" + repo.mkdir() + subprocess.run(["git", "init", "-b", "main"], cwd=repo, check=True, capture_output=True) + subprocess.run(["git", "config", "user.email", "test@example.com"], cwd=repo, check=True) + subprocess.run(["git", "config", "user.name", "Test User"], cwd=repo, check=True) + (repo / "README.md").write_text("test\n", encoding="utf-8") + subprocess.run(["git", "add", "README.md"], cwd=repo, check=True) + subprocess.run(["git", "commit", "-m", "init"], cwd=repo, check=True, capture_output=True) + + pending = { + "agent": "cursor", + "git_author": "Prakhar", + "model": "cursor-test", + "session_id": "sess-1", + "lines": {"src/app.py": [[1, 2]]}, + "prompt_excerpt": "add route guard", + "prompt_hash": "abc123", + "intent": "security hardening", + "files_read": ["src/auth.py", "src/config.py"], + "trust": 91, + "flags": ["security"], + "tool": "afterFileEdit", + } + + original = os.environ.get("HOME") + try: + os.environ["HOME"] = tmp + traces_path = self.mod.write_agent_trace( + str(repo), pending, "deadbeef", "2026-04-27T00:00:00Z" + ) + finally: + if original is not None: + os.environ["HOME"] = original + + self.assertIsNotNone(traces_path) + raw = Path(traces_path).read_text(encoding="utf-8").strip() + trace = json.loads(raw) + metadata = trace["metadata"]["agentdiff"] + self.assertEqual(metadata["intent"], "security hardening") + self.assertEqual(metadata["files_read"], ["src/auth.py", "src/config.py"]) + self.assertEqual(metadata["author"], "Prakhar") + self.assertEqual(metadata["capture_tool"], "afterFileEdit") + if __name__ == "__main__": unittest.main() diff --git a/scripts/tests/test_record_context.py b/scripts/tests/test_record_context.py new file mode 100644 index 0000000..44570ca --- /dev/null +++ b/scripts/tests/test_record_context.py @@ -0,0 +1,47 @@ +import json +import subprocess +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +RECORD_CONTEXT = REPO_ROOT / "scripts" / "record-context.py" + + +class RecordContextTests(unittest.TestCase): + def test_cli_flags_do_not_block_without_stdin_payload(self): + with tempfile.TemporaryDirectory() as tmp: + repo = Path(tmp) / "repo" + repo.mkdir() + subprocess.run(["git", "init", "-b", "main"], cwd=repo, check=True, capture_output=True) + + result = subprocess.run( + [ + "python3", + str(RECORD_CONTEXT), + "--cwd", + str(repo), + "--agent", + "cursor", + "--model-id", + "validation-model", + "--files-read", + '["src/app.py"]', + "--intent", + "context validation", + ], + text=True, + capture_output=True, + timeout=2, + ) + + self.assertEqual(result.returncode, 0) + pending = json.loads((repo / ".git" / "agentdiff" / "pending.json").read_text()) + self.assertEqual(pending["agent"], "cursor") + self.assertEqual(pending["model_id"], "validation-model") + self.assertEqual(pending["files_read"], ["src/app.py"]) + self.assertEqual(pending["intent"], "context validation") + + +if __name__ == "__main__": + unittest.main() diff --git a/src/cli.rs b/src/cli.rs index e20b306..d74110e 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -30,6 +30,9 @@ pub enum Command { /// Show line-level attribution for a file (like git-blame) Blame(BlameArgs), + /// Show agent context for a file + Context(ContextArgs), + /// Aggregate report in text, markdown, annotations, or JSONL format Report(ReportArgs), @@ -63,6 +66,9 @@ pub enum Command { /// Write agentdiff CI workflow files to .github/workflows/ InstallCi(InstallCiArgs), + /// Install the AgentDiff context skill for Cursor agents + InstallSkill(InstallSkillArgs), + /// [internal] Sign the last trace entry — called by the post-commit hook #[command(hide = true)] SignEntry, @@ -158,9 +164,27 @@ pub struct BlameArgs { pub agent: Option, } +#[derive(Args, Debug)] +pub struct ContextArgs { + /// File to explain (relative to repo root) + pub file: std::path::PathBuf, + + /// Output machine-readable JSON + #[arg(long)] + pub json: bool, + + /// Only include traces whose agent name contains this substring + #[arg(long)] + pub agent: Option, + + /// Limit number of trace records shown + #[arg(short = 'n', long, default_value_t = 10)] + pub limit: usize, +} + #[derive(Args, Debug)] pub struct ReportArgs { - /// Output format: text (default, terminal-friendly) | markdown | annotations | jsonl + /// Output format: text (default, terminal-friendly) | markdown | annotations | jsonl | json #[arg(long, default_value = "text")] pub format: ReportFormat, @@ -185,6 +209,10 @@ pub struct ReportArgs { #[arg(long)] pub model: Option, + /// Include structured intent/files-read context in markdown or JSON reports + #[arg(long)] + pub context: bool, + /// With --format=text: also show per-file breakdown #[arg(long)] pub by_file: bool, @@ -258,6 +286,8 @@ pub enum ReportFormat { Annotations, /// Agent Trace JSONL (replaces `export`) Jsonl, + /// Structured JSON summary + Json, } // ── Keys ───────────────────────────────────────────────────────────────────── @@ -331,3 +361,19 @@ pub struct InstallCiArgs { pub force: bool, } +#[derive(Args, Debug)] +pub struct InstallSkillArgs { + /// Where to install the skill: project writes .cursor/skills, global writes ~/.agents/skills + #[arg(long, default_value = "project")] + pub scope: SkillScope, + + /// Overwrite an existing skill file + #[arg(long)] + pub force: bool, +} + +#[derive(Debug, Clone, clap::ValueEnum, PartialEq, Eq)] +pub enum SkillScope { + Project, + Global, +} diff --git a/src/commands/context.rs b/src/commands/context.rs new file mode 100644 index 0000000..e2ad175 --- /dev/null +++ b/src/commands/context.rs @@ -0,0 +1,149 @@ +use crate::cli::ContextArgs; +use crate::data::{AgentTrace, TraceFile}; +use crate::store::Store; +use crate::util::{agent_color_str, print_command_header, print_not_initialized}; +use anyhow::Result; +use colored::Colorize; +use std::path::Path; + +pub fn run(store: &Store, args: &ContextArgs) -> Result<()> { + if !store.is_initialized() { + print_not_initialized(); + return Ok(()); + } + + let rel_path = normalize_path(&args.file); + let mut traces: Vec = store + .load_all_traces()? + .into_iter() + .filter(|trace| trace.files.iter().any(|file| file.path == rel_path)) + .collect(); + + if let Some(agent) = &args.agent { + traces.retain(|trace| trace.agent_name().contains(agent)); + } + + traces.sort_by(|a, b| b.timestamp.cmp(&a.timestamp)); + traces.truncate(args.limit); + + if args.json { + let body = context_json(&rel_path, &traces)?; + println!("{}", serde_json::to_string_pretty(&body)?); + return Ok(()); + } + + print_command_header("context"); + println!(" {}", rel_path.bold()); + println!(); + + if traces.is_empty() { + println!(" No AgentDiff context found for this file."); + println!(); + return Ok(()); + } + + for trace in &traces { + let meta = trace.agentdiff_metadata(); + let intent = meta + .as_ref() + .and_then(|m| m.intent.as_deref()) + .filter(|s| !s.trim().is_empty()) + .unwrap_or("unspecified"); + println!( + " {} {} {}", + short_id(&trace.id).dimmed(), + agent_color_str(trace.agent_name()), + trace.timestamp.to_rfc3339().dimmed() + ); + println!(" Intent: {}", intent); + if let Some(prompt) = meta + .as_ref() + .and_then(|m| m.prompt_excerpt.as_deref()) + .filter(|p| !p.trim().is_empty()) + { + println!(" Prompt: {}", prompt); + } + if let Some(files_read) = meta + .as_ref() + .map(|m| m.files_read.as_slice()) + .filter(|files| !files.is_empty()) + { + println!(" Files read: {}", files_read.join(", ")); + } + if let Some(flags) = meta + .as_ref() + .map(|m| m.flags.as_slice()) + .filter(|flags| !flags.is_empty()) + { + println!(" Flags: {}", flags.join(", ")); + } + for file in trace.files.iter().filter(|file| file.path == rel_path) { + println!(" Ranges: {}", format_ranges(file)); + } + println!(); + } + + Ok(()) +} + +fn normalize_path(path: &Path) -> String { + path.to_string_lossy().replace('\\', "/") +} + +fn context_json(file: &str, traces: &[AgentTrace]) -> Result { + let values: Vec<_> = traces + .iter() + .map(|trace| { + let meta = trace.agentdiff_metadata(); + serde_json::json!({ + "id": trace.id, + "short_id": short_id(&trace.id), + "timestamp": trace.timestamp, + "sha": trace.sha(), + "agent": trace.agent_name(), + "intent": meta.as_ref().and_then(|m| m.intent.clone()), + "prompt_excerpt": meta.as_ref().and_then(|m| m.prompt_excerpt.clone()), + "files_read": meta.as_ref().map(|m| m.files_read.clone()).unwrap_or_default(), + "flags": meta.as_ref().map(|m| m.flags.clone()).unwrap_or_default(), + "trust": meta.as_ref().and_then(|m| m.trust), + "ranges": trace.files.iter() + .filter(|f| f.path == file) + .flat_map(|f| f.conversations.iter()) + .flat_map(|c| c.ranges.iter()) + .map(|r| serde_json::json!({ + "start_line": r.start_line, + "end_line": r.end_line, + })) + .collect::>() + }) + }) + .collect(); + Ok(serde_json::json!({ + "file": file, + "traces": values, + })) +} + +fn format_ranges(file: &TraceFile) -> String { + let ranges: Vec = file + .conversations + .iter() + .flat_map(|conv| conv.ranges.iter()) + .map(|range| { + if range.start_line == range.end_line { + range.start_line.to_string() + } else { + format!("{}-{}", range.start_line, range.end_line) + } + }) + .collect(); + if ranges.is_empty() { + "unknown".to_string() + } else { + ranges.join(", ") + } +} + +fn short_id(id: &str) -> &str { + &id[..id.len().min(8)] +} diff --git a/src/commands/install_skill.rs b/src/commands/install_skill.rs new file mode 100644 index 0000000..ddf28f1 --- /dev/null +++ b/src/commands/install_skill.rs @@ -0,0 +1,88 @@ +use crate::cli::{InstallSkillArgs, SkillScope}; +use crate::util::{ok, print_command_header, warn}; +use anyhow::{Context, Result}; +use colored::Colorize; +use std::path::{Path, PathBuf}; + +const SKILL_CONTENT: &str = include_str!("../../.cursor/skills/agentdiff-context/SKILL.md"); +const SKILL_REL_PATH: &[&str] = &["agentdiff-context", "SKILL.md"]; + +pub fn run(repo_root: &Path, args: &InstallSkillArgs) -> Result<()> { + print_command_header("install-skill"); + + let path = skill_path(repo_root, &args.scope)?; + if path.exists() && !args.force { + println!( + " {} {} already exists — skipping (use --force to overwrite)", + warn(), + path.display() + ); + println!(); + return Ok(()); + } + + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .with_context(|| format!("creating {}", parent.display()))?; + } + std::fs::write(&path, SKILL_CONTENT).with_context(|| format!("writing {}", path.display()))?; + + println!(" {} wrote {}", ok(), path.display()); + println!(); + println!(" Next steps:"); + match args.scope { + SkillScope::Project => { + println!( + " 1. Commit {}", + ".cursor/skills/agentdiff-context/SKILL.md".cyan() + ); + println!(" 2. Ask agents to use AgentDiff context before editing traced files"); + } + SkillScope::Global => { + println!(" 1. Restart or refresh Cursor agents so the global skill is discovered"); + println!( + " 2. Prefer project scope when repository-specific guidance should be versioned" + ); + } + } + println!(); + Ok(()) +} + +fn skill_path(repo_root: &Path, scope: &SkillScope) -> Result { + match scope { + SkillScope::Project => Ok(repo_root + .join(".cursor") + .join("skills") + .join(SKILL_REL_PATH[0]) + .join(SKILL_REL_PATH[1])), + SkillScope::Global => { + let home = dirs::home_dir().context("could not determine home directory")?; + Ok(home + .join(".agents") + .join("skills") + .join(SKILL_REL_PATH[0]) + .join(SKILL_REL_PATH[1])) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn project_scope_writes_under_repo_cursor_skills() { + let path = skill_path(Path::new("/repo"), &SkillScope::Project).unwrap(); + assert_eq!( + path, + PathBuf::from("/repo/.cursor/skills/agentdiff-context/SKILL.md") + ); + } + + #[test] + fn embedded_skill_has_expected_frontmatter() { + assert!(SKILL_CONTENT.contains("name: agentdiff-context")); + assert!(SKILL_CONTENT.contains("agentdiff context path/to/file --json")); + } +} diff --git a/src/commands/mod.rs b/src/commands/mod.rs index ed2b50a..2034962 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1,8 +1,10 @@ pub mod blame; pub mod config_cmd; pub mod consolidate; -pub mod install_ci; +pub mod context; pub mod diff; +pub mod install_ci; +pub mod install_skill; pub mod keys; pub mod list; pub mod policy; diff --git a/src/commands/report.rs b/src/commands/report.rs index 9912361..e904cf4 100644 --- a/src/commands/report.rs +++ b/src/commands/report.rs @@ -1,5 +1,5 @@ use crate::cli::{ReportArgs, ReportFormat}; -use crate::data::Entry; +use crate::data::{AgentTrace, Entry}; use crate::store::Store; use crate::util::{agent_color_str, print_command_header, print_not_initialized}; use anyhow::{Context, Result}; @@ -8,6 +8,7 @@ use std::collections::{HashMap, HashSet}; use std::fs; use std::io::{self, Write}; use std::path::Path; +use std::process::{Command, Stdio}; pub fn run(store: &Store, args: &ReportArgs) -> Result<()> { // Text format reads like the old `stats` — needs initialization check + header. @@ -22,33 +23,48 @@ pub fn run(store: &Store, args: &ReportArgs) -> Result<()> { } let mut entries = store.load_entries()?; + let mut traces = store.load_all_traces()?; if let Some(ref since) = args.since { if let Ok(since_ts) = chrono::DateTime::parse_from_rfc3339(since).map(|dt| dt.with_timezone(&chrono::Utc)) { entries.retain(|e| e.timestamp >= since_ts); + traces.retain(|t| t.timestamp >= since_ts); } } if let Some(ref agent) = args.agent { entries.retain(|e| e.agent.contains(agent.as_str())); + traces.retain(|t| t.agent_name().contains(agent.as_str())); } if let Some(ref model) = args.model { entries.retain(|e| e.model.contains(model.as_str())); + traces.retain(|t| trace_models(t).iter().any(|m| m.contains(model.as_str()))); } // --post-pr-comment: generate markdown and post via gh CLI regardless of --format. if let Some(ref pr_arg) = args.post_pr_comment { - let md = markdown_report(&entries)?; + if let Some(pr_shas) = current_branch_commits(&store.repo_root) { + traces = filter_traces_by_commit_set(&traces, &pr_shas); + } + let md = markdown_trace_report(&traces, true)?; return post_pr_comment(&store.repo_root, &md, *pr_arg); } match args.format { ReportFormat::Text => run_text(store, &entries, args), ReportFormat::Markdown => { - let md = markdown_report(&entries)?; + let md = if args.context { + markdown_trace_report(&traces, true)? + } else { + markdown_report(&entries)? + }; write_or_stdout(args.out.as_deref(), &md) } + ReportFormat::Json => { + let json = serde_json::to_string_pretty(&context_json_report(&traces)?)?; + write_or_stdout(args.out.as_deref(), &format!("{json}\n")) + } ReportFormat::Annotations => { let json = format_annotations(&entries, args.out.is_none())?; write_or_stdout(args.out.as_deref(), &json) @@ -58,8 +74,31 @@ pub fn run(store: &Store, args: &ReportArgs) -> Result<()> { } fn post_pr_comment(repo_root: &Path, body: &str, pr_number: Option) -> Result<()> { - use std::process::{Command, Stdio}; + let edit = run_gh_pr_comment(repo_root, body, pr_number, true) + .context("running gh pr comment --edit-last")?; + if edit.status.success() { + return Ok(()); + } + + let create = + run_gh_pr_comment(repo_root, body, pr_number, false).context("running gh pr comment")?; + if create.status.success() { + return Ok(()); + } + + anyhow::bail!( + "gh pr comment failed; edit-last stderr: {}; create stderr: {}", + String::from_utf8_lossy(&edit.stderr).trim(), + String::from_utf8_lossy(&create.stderr).trim() + ); +} +fn run_gh_pr_comment( + repo_root: &Path, + body: &str, + pr_number: Option, + edit_last: bool, +) -> Result { let mut cmd = Command::new("gh"); cmd.arg("pr").arg("comment"); @@ -67,17 +106,90 @@ fn post_pr_comment(repo_root: &Path, body: &str, pr_number: Option) -> Resu cmd.arg(n.to_string()); } - cmd.args(["--body", body, "--edit-last", "--create-if-none"]) - .current_dir(repo_root) + cmd.args(["--body", body]); + if edit_last { + cmd.arg("--edit-last"); + } + cmd.current_dir(repo_root) .stdin(Stdio::null()) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()); + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output() + .context("running gh pr comment") +} - let status = cmd.status().context("running gh pr comment")?; - if !status.success() { - anyhow::bail!("gh pr comment failed (exit {})", status); +fn current_branch_commits(repo_root: &Path) -> Option> { + let base = find_merge_base(repo_root)?; + let out = std::process::Command::new("git") + .args(["rev-list", &format!("{base}..HEAD")]) + .current_dir(repo_root) + .output() + .ok()?; + if !out.status.success() { + return None; } - Ok(()) + let shas = String::from_utf8_lossy(&out.stdout) + .lines() + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string) + .collect(); + Some(shas) +} + +fn find_merge_base(repo_root: &Path) -> Option { + let mut candidates = Vec::new(); + if let Some(remote_head) = remote_default_branch(repo_root) { + candidates.push(remote_head); + } + for branch in ["origin/main", "origin/master", "main", "master"] { + if !candidates.iter().any(|b| b == branch) { + candidates.push(branch.to_string()); + } + } + + for branch in candidates { + let out = std::process::Command::new("git") + .args(["merge-base", "HEAD", &branch]) + .current_dir(repo_root) + .output() + .ok()?; + if out.status.success() { + let sha = String::from_utf8(out.stdout).ok()?.trim().to_string(); + if !sha.is_empty() { + return Some(sha); + } + } + } + None +} + +fn remote_default_branch(repo_root: &Path) -> Option { + let out = std::process::Command::new("git") + .args(["symbolic-ref", "refs/remotes/origin/HEAD"]) + .current_dir(repo_root) + .output() + .ok()?; + if !out.status.success() { + return None; + } + let branch = String::from_utf8(out.stdout).ok()?.trim().to_string(); + if branch.is_empty() { + None + } else { + Some(branch.trim_start_matches("refs/remotes/").to_string()) + } +} + +fn filter_traces_by_commit_set( + traces: &[AgentTrace], + commit_shas: &HashSet, +) -> Vec { + traces + .iter() + .filter(|trace| commit_shas.contains(trace.sha())) + .cloned() + .collect() } // ── Text (replaces `stats`) ────────────────────────────────────────────────── @@ -295,6 +407,287 @@ fn markdown_report(entries: &[Entry]) -> Result { Ok(out) } +#[derive(Default)] +struct ContextGroup { + intent: String, + lines: usize, + files: HashSet, + agents: HashSet, + models: HashSet, + files_read: HashSet, + prompts: HashSet, + flags: HashSet, + trace_ids: Vec, + max_trust: Option, +} + +#[derive(Default)] +struct FileContextRow { + lines: usize, + agents: HashMap, + intents: HashSet, + trace_ids: Vec, +} + +fn markdown_trace_report(traces: &[AgentTrace], include_context: bool) -> Result { + let mut out = String::from("# AgentDiff Report\n\n"); + if traces.is_empty() { + out.push_str("No AgentDiff traces found.\n"); + return Ok(out); + } + + let mut agent_counts: HashMap = HashMap::new(); + let mut file_rows: HashMap = HashMap::new(); + let mut context_groups: HashMap = HashMap::new(); + let mut total_lines = 0usize; + + for trace in traces { + let agent = trace.agent_name().to_string(); + let meta = trace.agentdiff_metadata(); + let intent = meta + .as_ref() + .and_then(|m| m.intent.clone()) + .filter(|s| !s.trim().is_empty()) + .unwrap_or_else(|| "unspecified".to_string()); + let line_count = trace_line_count(trace); + total_lines += line_count; + *agent_counts.entry(agent.clone()).or_default() += line_count; + + let group = context_groups + .entry(intent.clone()) + .or_insert_with(|| ContextGroup { + intent: intent.clone(), + ..Default::default() + }); + group.lines += line_count; + group.agents.insert(agent.clone()); + group.trace_ids.push(short_id(&trace.id).to_string()); + + for model in trace_models(trace) { + if !model.is_empty() { + group.models.insert(model); + } + } + + if let Some(meta) = meta.as_ref() { + for file in &meta.files_read { + group.files_read.insert(file.clone()); + } + for flag in &meta.flags { + group.flags.insert(flag.clone()); + } + if let Some(prompt) = meta + .prompt_excerpt + .as_ref() + .filter(|p| !p.trim().is_empty()) + { + group.prompts.insert(prompt.clone()); + } + if let Some(trust) = meta.trust { + group.max_trust = Some(group.max_trust.map_or(trust, |v| v.max(trust))); + } + } + + for file in &trace.files { + group.files.insert(file.path.clone()); + let file_line_count = trace_file_line_count(file); + let row = file_rows.entry(file.path.clone()).or_default(); + row.lines += file_line_count; + *row.agents.entry(agent.clone()).or_default() += file_line_count; + row.intents.insert(intent.clone()); + row.trace_ids.push(short_id(&trace.id).to_string()); + } + } + + out.push_str("## Summary\n\n"); + out.push_str("| Agent | Lines | % |\n"); + out.push_str("|-------|-------|---|\n"); + let mut agents: Vec<_> = agent_counts.iter().collect(); + agents.sort_by(|a, b| b.1.cmp(a.1).then_with(|| a.0.cmp(b.0))); + for (agent, lines) in agents { + let pct = if total_lines > 0 { + (*lines as f64 / total_lines as f64 * 100.0).round() as u32 + } else { + 0 + }; + out.push_str(&format!("| {} | {} | {}% |\n", md_cell(agent), lines, pct)); + } + + if include_context { + out.push_str("\n## Review Context\n\n"); + let mut groups: Vec<_> = context_groups.values().collect(); + groups.sort_by(|a, b| b.lines.cmp(&a.lines).then_with(|| a.intent.cmp(&b.intent))); + for group in groups.iter().take(5) { + out.push_str(&format!( + "- Intent: {} ({} lines, {} file{})\n", + group.intent, + group.lines, + group.files.len(), + if group.files.len() == 1 { "" } else { "s" } + )); + out.push_str(&format!( + " - Agent/model: {} / {}\n", + limited_join(&group.agents, 4), + limited_join(&group.models, 4) + )); + if !group.files_read.is_empty() { + out.push_str(&format!( + " - Files read: {}\n", + limited_join(&group.files_read, 6) + )); + } + if !group.prompts.is_empty() { + out.push_str(&format!( + " - Prompt: {}\n", + limited_join(&group.prompts, 2) + )); + } + if !group.flags.is_empty() { + out.push_str(&format!(" - Flags: {}\n", limited_join(&group.flags, 6))); + } + if let Some(trust) = group.max_trust { + out.push_str(&format!(" - Trust: {trust}\n")); + } + } + } + + out.push_str("\n## Files To Review First\n\n"); + out.push_str("| File | Lines | Dominant Agent | Intent | Context |\n"); + out.push_str("|------|-------|----------------|--------|---------|\n"); + let mut rows: Vec<_> = file_rows.into_iter().collect(); + rows.sort_by(|a, b| b.1.lines.cmp(&a.1.lines).then_with(|| a.0.cmp(&b.0))); + for (file, row) in rows.iter().take(20) { + let dominant = row + .agents + .iter() + .max_by(|a, b| a.1.cmp(b.1).then_with(|| b.0.cmp(a.0))) + .map(|(agent, _)| agent.as_str()) + .unwrap_or("unknown"); + let context = if row.trace_ids.is_empty() { + String::new() + } else { + format!("trace {}", row.trace_ids[0]) + }; + out.push_str(&format!( + "| {} | {} | {} | {} | {} |\n", + md_cell(file), + row.lines, + md_cell(dominant), + md_cell(&limited_join(&row.intents, 3)), + md_cell(&context) + )); + } + + if include_context { + out.push_str("\n
\nTrace details\n\n"); + out.push_str("| Trace | Agent | Intent | Files | Lines |\n"); + out.push_str("|-------|-------|--------|-------|-------|\n"); + for trace in traces.iter().take(30) { + let meta = trace.agentdiff_metadata(); + let intent = meta + .as_ref() + .and_then(|m| m.intent.clone()) + .filter(|s| !s.trim().is_empty()) + .unwrap_or_else(|| "unspecified".to_string()); + let files: HashSet = trace.files.iter().map(|f| f.path.clone()).collect(); + out.push_str(&format!( + "| {} | {} | {} | {} | {} |\n", + short_id(&trace.id), + md_cell(trace.agent_name()), + md_cell(&intent), + md_cell(&limited_join(&files, 5)), + trace_line_count(trace) + )); + } + out.push_str("\n
\n"); + } + + Ok(out) +} + +fn context_json_report(traces: &[AgentTrace]) -> Result { + let total_lines: usize = traces.iter().map(trace_line_count).sum(); + let trace_values: Vec<_> = traces + .iter() + .map(|trace| { + let meta = trace.agentdiff_metadata(); + serde_json::json!({ + "id": trace.id, + "short_id": short_id(&trace.id), + "timestamp": trace.timestamp, + "sha": trace.sha(), + "agent": trace.agent_name(), + "intent": meta.as_ref().and_then(|m| m.intent.clone()), + "prompt_excerpt": meta.as_ref().and_then(|m| m.prompt_excerpt.clone()), + "files_read": meta.as_ref().map(|m| m.files_read.clone()).unwrap_or_default(), + "flags": meta.as_ref().map(|m| m.flags.clone()).unwrap_or_default(), + "trust": meta.as_ref().and_then(|m| m.trust), + "files": trace.files.iter().map(|file| serde_json::json!({ + "path": file.path, + "lines": trace_file_line_count(file), + })).collect::>(), + }) + }) + .collect(); + Ok(serde_json::json!({ + "total_traces": traces.len(), + "total_lines": total_lines, + "traces": trace_values, + })) +} + +fn trace_models(trace: &AgentTrace) -> Vec { + let mut models = HashSet::new(); + for file in &trace.files { + for conv in &file.conversations { + if let Some(model) = &conv.contributor.model_id { + models.insert(model.clone()); + } + } + } + let mut out: Vec<_> = models.into_iter().collect(); + out.sort(); + out +} + +fn trace_line_count(trace: &AgentTrace) -> usize { + trace.files.iter().map(trace_file_line_count).sum() +} + +fn trace_file_line_count(file: &crate::data::TraceFile) -> usize { + file.conversations + .iter() + .flat_map(|conv| conv.ranges.iter()) + .map(|range| { + let lo = range.start_line.min(range.end_line); + let hi = range.start_line.max(range.end_line); + hi.saturating_sub(lo) as usize + 1 + }) + .sum() +} + +fn short_id(id: &str) -> &str { + &id[..id.len().min(8)] +} + +fn md_cell(value: &str) -> String { + value.replace('|', "\\|").replace('\n', " ") +} + +fn limited_join(values: &HashSet, limit: usize) -> String { + let mut sorted: Vec<_> = values.iter().filter(|v| !v.is_empty()).cloned().collect(); + sorted.sort(); + if sorted.is_empty() { + return "unknown".to_string(); + } + let extra = sorted.len().saturating_sub(limit); + let mut shown: Vec<_> = sorted.into_iter().take(limit).collect(); + if extra > 0 { + shown.push(format!("+{extra} more")); + } + shown.join(", ") +} + /// Per file: total attributed lines and agent with the largest share (ties → lexicographically smallest agent name). fn aggregate_file_dominant(entries: &[Entry]) -> Vec<(String, usize, String)> { let mut totals: HashMap = HashMap::new(); @@ -366,6 +759,9 @@ fn format_annotations(entries: &[Entry], for_stdout: bool) -> Result { #[cfg(test)] mod tests { use super::*; + use crate::data::{ + AgentTrace, Contributor, Conversation, ToolInfo, TraceFile, TraceRange, VcsInfo, + }; use chrono::Utc; fn sample_entry(agent: &str, file: &str, lines: Vec) -> Entry { @@ -392,6 +788,50 @@ mod tests { } } + fn sample_trace_with_context() -> AgentTrace { + AgentTrace { + version: "0.1.0".to_string(), + id: "550e8400-e29b-41d4-a716-446655440000".to_string(), + timestamp: Utc::now(), + vcs: Some(VcsInfo { + vcs_type: "git".to_string(), + revision: "abc123".to_string(), + }), + tool: Some(ToolInfo { + name: "cursor".to_string(), + version: None, + }), + files: vec![TraceFile { + path: "src/api.rs".to_string(), + conversations: vec![Conversation { + url: None, + contributor: Contributor { + contributor_type: "ai".to_string(), + model_id: Some("cursor-test".to_string()), + }, + ranges: vec![TraceRange { + start_line: 1, + end_line: 3, + content_hash: None, + contributor: None, + }], + related: None, + }], + }], + metadata: Some(serde_json::json!({ + "agentdiff": { + "intent": "security hardening", + "prompt_excerpt": "add route guard", + "files_read": ["src/auth.rs"], + "flags": ["security"], + "trust": 91, + "session_id": "sess-1" + } + })), + sig: None, + } + } + #[test] fn dominant_agent_picks_highest_line_share() { let entries = vec![ @@ -413,4 +853,35 @@ mod tests { let rows = aggregate_file_dominant(&entries); assert_eq!(rows[0].2, "alpha"); } + + #[test] + fn markdown_trace_report_includes_review_context() { + let md = markdown_trace_report(&[sample_trace_with_context()], true).unwrap(); + assert!(md.contains("## Review Context")); + assert!(md.contains("Intent: security hardening")); + assert!(md.contains("Files read: src/auth.rs")); + assert!(md.contains("| src/api.rs | 3 | cursor | security hardening | trace 550e8400 |")); + } + + #[test] + fn context_json_report_includes_trace_metadata() { + let json = context_json_report(&[sample_trace_with_context()]).unwrap(); + assert_eq!(json["total_traces"], 1); + assert_eq!(json["traces"][0]["intent"], "security hardening"); + assert_eq!(json["traces"][0]["files_read"][0], "src/auth.rs"); + } + + #[test] + fn filter_traces_by_commit_set_keeps_only_branch_commits() { + let mut keep = sample_trace_with_context(); + keep.vcs.as_mut().unwrap().revision = "keep-sha".to_string(); + let mut drop = sample_trace_with_context(); + drop.vcs.as_mut().unwrap().revision = "drop-sha".to_string(); + + let shas = HashSet::from(["keep-sha".to_string()]); + let filtered = filter_traces_by_commit_set(&[keep, drop], &shas); + + assert_eq!(filtered.len(), 1); + assert_eq!(filtered[0].sha(), "keep-sha"); + } } diff --git a/src/main.rs b/src/main.rs index 3eb5da6..bcd4734 100644 --- a/src/main.rs +++ b/src/main.rs @@ -26,7 +26,11 @@ fn main() -> anyhow::Result<()> { // Commands that don't require a git repository. let no_repo_needed = matches!( &cli.command, - Command::Config(_) | Command::Configure(_) | Command::Keys(_) | Command::InstallCi(_) + Command::Config(_) + | Command::Configure(_) + | Command::Keys(_) + | Command::InstallCi(_) + | Command::InstallSkill(_) ); // Resolve repo root @@ -70,6 +74,7 @@ fn main() -> anyhow::Result<()> { } Command::List(args) => commands::list::run(&store, &args), Command::Blame(args) => commands::blame::run(&store, &args), + Command::Context(args) => commands::context::run(&store, &args), Command::Report(args) => commands::report::run(&store, &args), Command::Diff(args) => commands::diff::run(&store, &args), Command::Show(args) => commands::show::run(&store, &args), @@ -85,6 +90,7 @@ fn main() -> anyhow::Result<()> { Command::Push(args) => commands::push::run(&store, &args), Command::Consolidate(args) => commands::consolidate::run(&store, &args), Command::InstallCi(args) => commands::install_ci::run(&repo_root, &args), + Command::InstallSkill(args) => commands::install_skill::run(&repo_root, &args), Command::SignEntry => commands::sign_entry::run(&store), } } diff --git a/src/store.rs b/src/store.rs index f44fcb3..f239b9e 100644 --- a/src/store.rs +++ b/src/store.rs @@ -91,7 +91,11 @@ impl Store { // Dedup by id field let mut seen = std::collections::HashSet::new(); values.retain(|v| { - let id = v.get("id").and_then(|v| v.as_str()).unwrap_or("").to_string(); + let id = v + .get("id") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); if id.is_empty() { true } else { @@ -290,9 +294,7 @@ pub fn write_to_ref( "git mktree failed: {}", String::from_utf8_lossy(&tree_out.stderr) ); - let tree_sha = String::from_utf8_lossy(&tree_out.stdout) - .trim() - .to_string(); + let tree_sha = String::from_utf8_lossy(&tree_out.stdout).trim().to_string(); // Find parent commit if ref already exists. let parent = Command::new("git") @@ -325,9 +327,7 @@ pub fn write_to_ref( "git commit-tree failed: {}", String::from_utf8_lossy(&commit.stderr) ); - let commit_sha = String::from_utf8_lossy(&commit.stdout) - .trim() - .to_string(); + let commit_sha = String::from_utf8_lossy(&commit.stdout).trim().to_string(); // Update the ref. let update = Command::new("git") @@ -390,8 +390,21 @@ fn get_github_nwo(repo_root: &Path) -> Result<(String, String)> { .context("git remote get-url origin")?; let remote_url = String::from_utf8_lossy(&out.stdout).trim().to_string(); let host = github_host(); - parse_github_nwo(&remote_url, &host) - .ok_or_else(|| anyhow::anyhow!("origin remote is not a GitHub URL (host={host}): {remote_url}")) + parse_github_nwo(&remote_url, &host).ok_or_else(|| { + anyhow::anyhow!("origin remote is not a GitHub URL (host={host}): {remote_url}") + }) +} + +/// Convert a git ref into the path fragment expected by GitHub's ref API. +/// +/// Per-branch agentdiff refs contain a literal `%2F` when the source branch +/// has `/` in its name. Percent signs must be escaped in the URL path; +/// otherwise GitHub decodes `%2F` as a real slash and looks up a different ref. +fn api_ref_path(ref_name: &str) -> String { + ref_name + .strip_prefix("refs/") + .unwrap_or(ref_name) + .replace('%', "%25") } /// Push JSONL content to a GitHub ref using the Git Database API. @@ -417,7 +430,7 @@ pub fn push_content_to_ref( let (owner, repo) = get_github_nwo(repo_root)?; // API ref format strips "refs/" prefix - let api_ref = ref_name.strip_prefix("refs/").unwrap_or(ref_name); + let api_ref = api_ref_path(ref_name); fn gh_api_json( owner: &str, @@ -448,8 +461,8 @@ pub fn push_content_to_ref( let err = String::from_utf8_lossy(&out.stderr); anyhow::bail!("gh api {method} {full_path} failed: {err}"); } - let v: serde_json::Value = serde_json::from_slice(&out.stdout) - .context("parsing gh api response")?; + let v: serde_json::Value = + serde_json::from_slice(&out.stdout).context("parsing gh api response")?; Ok(v) } @@ -472,7 +485,10 @@ pub fn push_content_to_ref( // Create the blob once — it's content-addressed, so this is idempotent. let encoded = base64::engine::general_purpose::STANDARD.encode(content.as_bytes()); let blob_resp = gh_api_json( - &owner, &repo, "POST", "git/blobs", + &owner, + &repo, + "POST", + "git/blobs", &serde_json::json!({"content": encoded, "encoding": "base64"}), repo_root, )?; @@ -487,7 +503,7 @@ pub fn push_content_to_ref( // Backoff: 200ms, 400ms, 800ms, 1.6s, 3.2s … capped at 5s per attempt. const MAX_RETRIES: u32 = 10; for attempt in 0..MAX_RETRIES { - let parent_sha = fetch_ref_sha(&owner, &repo, api_ref, repo_root); + let parent_sha = fetch_ref_sha(&owner, &repo, &api_ref, repo_root); // Build tree let tree_body = serde_json::json!({ @@ -504,8 +520,14 @@ pub fn push_content_to_ref( if let Some(ref p) = parent_sha { commit_body["parents"] = serde_json::json!([p]); } - let commit_resp = - gh_api_json(&owner, &repo, "POST", "git/commits", &commit_body, repo_root)?; + let commit_resp = gh_api_json( + &owner, + &repo, + "POST", + "git/commits", + &commit_body, + repo_root, + )?; let commit_sha = commit_resp["sha"] .as_str() .ok_or_else(|| anyhow::anyhow!("no sha in commit response"))? @@ -514,14 +536,18 @@ pub fn push_content_to_ref( // Update or create the ref (no force — CAS semantics). let ref_result = if parent_sha.is_some() { gh_api_json( - &owner, &repo, "PATCH", + &owner, + &repo, + "PATCH", &format!("git/refs/{api_ref}"), &serde_json::json!({"sha": commit_sha}), repo_root, ) } else { gh_api_json( - &owner, &repo, "POST", + &owner, + &repo, + "POST", "git/refs", &serde_json::json!({"ref": ref_name, "sha": commit_sha}), repo_root, @@ -532,7 +558,8 @@ pub fn push_content_to_ref( Ok(_) => return Ok(()), Err(e) => { let msg = e.to_string(); - let is_conflict = msg.contains("422") || msg.contains("not a fast forward") + let is_conflict = msg.contains("422") + || msg.contains("not a fast forward") || msg.contains("non-fast-forward"); if is_conflict && attempt < MAX_RETRIES - 1 { // Exponential backoff capped at 5s: 200ms, 400ms, 800ms … 5000ms @@ -559,7 +586,7 @@ pub fn fetch_ref_content_via_api( use std::process::{Command, Stdio}; let (owner, repo) = get_github_nwo(repo_root)?; - let api_ref = ref_name.strip_prefix("refs/").unwrap_or(ref_name); + let api_ref = api_ref_path(ref_name); // Get ref → commit SHA let ref_out = Command::new("gh") @@ -571,8 +598,8 @@ pub fn fetch_ref_content_via_api( if !ref_out.status.success() { return Ok(None); // ref doesn't exist } - let ref_json: serde_json::Value = serde_json::from_slice(&ref_out.stdout) - .unwrap_or(serde_json::Value::Null); + let ref_json: serde_json::Value = + serde_json::from_slice(&ref_out.stdout).unwrap_or(serde_json::Value::Null); let commit_sha = match ref_json["object"]["sha"].as_str() { Some(s) => s.to_string(), None => return Ok(None), @@ -580,7 +607,10 @@ pub fn fetch_ref_content_via_api( // Get commit → tree SHA let commit_out = Command::new("gh") - .args(["api", &format!("/repos/{owner}/{repo}/git/commits/{commit_sha}")]) + .args([ + "api", + &format!("/repos/{owner}/{repo}/git/commits/{commit_sha}"), + ]) .current_dir(repo_root) .stdout(Stdio::piped()) .stderr(Stdio::null()) @@ -588,8 +618,8 @@ pub fn fetch_ref_content_via_api( if !commit_out.status.success() { return Ok(None); } - let commit_json: serde_json::Value = serde_json::from_slice(&commit_out.stdout) - .unwrap_or(serde_json::Value::Null); + let commit_json: serde_json::Value = + serde_json::from_slice(&commit_out.stdout).unwrap_or(serde_json::Value::Null); let tree_sha = match commit_json["tree"]["sha"].as_str() { Some(s) => s.to_string(), None => return Ok(None), @@ -597,7 +627,10 @@ pub fn fetch_ref_content_via_api( // Get tree → blob SHA for the file let tree_out = Command::new("gh") - .args(["api", &format!("/repos/{owner}/{repo}/git/trees/{tree_sha}")]) + .args([ + "api", + &format!("/repos/{owner}/{repo}/git/trees/{tree_sha}"), + ]) .current_dir(repo_root) .stdout(Stdio::piped()) .stderr(Stdio::null()) @@ -605,16 +638,14 @@ pub fn fetch_ref_content_via_api( if !tree_out.status.success() { return Ok(None); } - let tree_json: serde_json::Value = serde_json::from_slice(&tree_out.stdout) - .unwrap_or(serde_json::Value::Null); - let blob_sha = tree_json["tree"] - .as_array() - .and_then(|arr| { - arr.iter() - .find(|e| e["path"].as_str() == Some(filename)) - .and_then(|e| e["sha"].as_str()) - .map(String::from) - }); + let tree_json: serde_json::Value = + serde_json::from_slice(&tree_out.stdout).unwrap_or(serde_json::Value::Null); + let blob_sha = tree_json["tree"].as_array().and_then(|arr| { + arr.iter() + .find(|e| e["path"].as_str() == Some(filename)) + .and_then(|e| e["sha"].as_str()) + .map(String::from) + }); let blob_sha = match blob_sha { Some(s) => s, None => return Ok(None), @@ -622,7 +653,10 @@ pub fn fetch_ref_content_via_api( // Get blob content (base64 decoded) let blob_out = Command::new("gh") - .args(["api", &format!("/repos/{owner}/{repo}/git/blobs/{blob_sha}")]) + .args([ + "api", + &format!("/repos/{owner}/{repo}/git/blobs/{blob_sha}"), + ]) .current_dir(repo_root) .stdout(Stdio::piped()) .stderr(Stdio::null()) @@ -630,8 +664,8 @@ pub fn fetch_ref_content_via_api( if !blob_out.status.success() { return Ok(None); } - let blob_json: serde_json::Value = serde_json::from_slice(&blob_out.stdout) - .unwrap_or(serde_json::Value::Null); + let blob_json: serde_json::Value = + serde_json::from_slice(&blob_out.stdout).unwrap_or(serde_json::Value::Null); let encoded = match blob_json["content"].as_str() { Some(s) => s.replace('\n', ""), // GitHub wraps at 60 chars None => return Ok(None), @@ -648,7 +682,7 @@ pub fn delete_remote_ref(repo_root: &Path, ref_name: &str) -> Result<()> { use std::process::{Command, Stdio}; let (owner, repo) = get_github_nwo(repo_root)?; - let api_ref = ref_name.strip_prefix("refs/").unwrap_or(ref_name); + let api_ref = api_ref_path(ref_name); let api_path = format!("/repos/{owner}/{repo}/git/refs/{api_ref}"); let out = Command::new("gh") @@ -671,10 +705,7 @@ pub fn delete_remote_ref(repo_root: &Path, ref_name: &str) -> Result<()> { /// Convert a branch name to a per-branch ref path. /// e.g. "feature/auth" → "refs/agentdiff/traces/feature--auth" pub fn branch_ref_name(branch: &str) -> String { - format!( - "refs/agentdiff/traces/{}", - sanitize_branch_name(branch) - ) + format!("refs/agentdiff/traces/{}", sanitize_branch_name(branch)) } /// Sanitize branch name for use in ref paths and local file names. @@ -757,6 +788,27 @@ pub fn traces_to_jsonl(traces: &[AgentTrace]) -> Result { Ok(out) } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn api_ref_path_escapes_literal_percent_encoded_branch_slash() { + assert_eq!( + api_ref_path("refs/agentdiff/traces/codex%2Fmisc"), + "agentdiff/traces/codex%252Fmisc" + ); + } + + #[test] + fn api_ref_path_keeps_real_ref_path_slashes() { + assert_eq!( + api_ref_path("refs/heads/feature/example"), + "heads/feature/example" + ); + } +} + fn load_session_from(path: &Path, out: &mut Vec, committed: bool) -> Result<()> { if !path.exists() { return Ok(());