diff --git a/README.md b/README.md index 2f5f447..51a6474 100644 --- a/README.md +++ b/README.md @@ -128,6 +128,9 @@ aifr list --depth -1 --type f . # Git refs and log aifr refs --branches --tags aifr log --max-count 5 +aifr log --oneline --max-count 10 +aifr log --format=text --divider=xml --max-count 3 +aifr log --verbose --max-count 1 # tree hash, parent hashes, committer # Compare files across refs aifr diff HEAD~1:README.md README.md @@ -222,7 +225,7 @@ Run `aifr sensitive` to see the full pattern list. | `hexdump` | Hex dump of file contents | | `checksum` | Compute file checksums | | `refs` | List git branches, tags, remotes | -| `log` | Git commit log with files changed | +| `log` | Git commit log (text/oneline/xml/json, verbose mode) | | `reflog` | Show git reflog for a ref | | `stash-list` | List git stashes | | `rev-parse` | Resolve a git ref to a commit hash | diff --git a/cmd/aifr/cmd_log.go b/cmd/aifr/cmd_log.go index 28e530a..6452531 100644 --- a/cmd/aifr/cmd_log.go +++ b/cmd/aifr/cmd_log.go @@ -11,12 +11,28 @@ import ( var ( logMaxCount int + logSkip int + logOneline bool + logDivider string + logVerbose bool ) var logCmd = &cobra.Command{ Use: "log [repo|path][:]", Short: "Git commit log", - Args: cobra.MaximumNArgs(1), + Long: `Show git commit log with structured entries. + +Output formats for --format text: + default git-log style with commit/Author/Date headers + --oneline compact one-line-per-commit (hash + subject) + +Divider formats for --format text (ignored with --oneline): + plain git-log style (default) + xml XML-tagged output with escaped content + +Use --verbose to include tree hash, parent hashes, and committer +details (when they differ from the author) in JSON output.`, + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { eng, err := buildEngine() if err != nil { @@ -46,7 +62,16 @@ var logCmd = &cobra.Command{ } } - resp, err := eng.Log(repoName, ref, engine.LogParams{MaxCount: logMaxCount}) + // --oneline implies text format. + if logOneline { + flagFormat = "oneline" + } + + resp, err := eng.Log(repoName, ref, engine.LogParams{ + MaxCount: logMaxCount, + Skip: logSkip, + Verbose: logVerbose, + }) if err != nil { exitWithError(err) return nil @@ -58,5 +83,9 @@ var logCmd = &cobra.Command{ func init() { logCmd.Flags().IntVar(&logMaxCount, "max-count", 20, "maximum commits to show") + logCmd.Flags().IntVar(&logSkip, "skip", 0, "skip this many commits before showing results") + logCmd.Flags().BoolVar(&logOneline, "oneline", false, "compact one-line-per-commit output") + logCmd.Flags().StringVar(&logDivider, "divider", "plain", "divider format for text output: plain, xml") + logCmd.Flags().BoolVar(&logVerbose, "verbose", false, "include tree hash, parent hashes, committer details") rootCmd.AddCommand(logCmd) } diff --git a/cmd/aifr/main.go b/cmd/aifr/main.go index 0c8a443..1713a63 100644 --- a/cmd/aifr/main.go +++ b/cmd/aifr/main.go @@ -121,10 +121,10 @@ func writeJSON(v any) { // writeOutput writes the response in the selected format. func writeOutput(v any) { - if flagNumberLines && flagFormat != "text" { + if flagNumberLines && flagFormat != "text" && flagFormat != "oneline" { applyNumberLines(v) } - if flagFormat != "text" { + if flagFormat != "text" && flagFormat != "oneline" { writeJSON(v) return } @@ -143,7 +143,14 @@ func writeOutput(v any) { case *protocol.DiffResponse: output.WriteDiffText(w, resp) case *protocol.LogResponse: - output.WriteLogText(w, resp) + switch { + case flagFormat == "oneline": + output.WriteLogOneline(w, resp) + case logDivider == "xml": + output.WriteLogXML(w, resp) + default: + output.WriteLogText(w, resp) + } case *protocol.RefsResponse: output.WriteRefsText(w, resp) case *protocol.WcResponse: @@ -200,10 +207,14 @@ func applyNumberLines(v any) { // cliSupportedFormats returns the list of output formats a command supports. func cliSupportedFormats(cmd *cobra.Command) []string { - if cmd.Name() == "version" { + switch cmd.Name() { + case "version": return []string{"json", "text", "short"} + case "log": + return []string{"json", "text", "oneline"} + default: + return []string{"json", "text"} } - return []string{"json", "text"} } // loadConfig loads the effective configuration. diff --git a/internal/engine/gitops.go b/internal/engine/gitops.go index 26215ce..ab602ed 100644 --- a/internal/engine/gitops.go +++ b/internal/engine/gitops.go @@ -8,6 +8,7 @@ import ( "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/utils/merkletrie" "go.pennock.tech/aifr/internal/gitprovider" "go.pennock.tech/aifr/pkg/protocol" @@ -271,7 +272,9 @@ func (e *Engine) Refs(repoName string, branches, tags, remotes bool) (*protocol. // LogParams controls git log queries. type LogParams struct { MaxCount int // 0 = default (20) + Skip int // skip this many commits before collecting entries StartHash string // start from this commit's parent (for pagination) + Verbose bool // include tree hash, parent hashes, committer details } // Log returns git commit log entries. @@ -320,9 +323,23 @@ func (e *Engine) Log(repoName, ref string, params LogParams) (*protocol.LogRespo } } + // Skip commits if requested. + for skipped := 0; skipped < params.Skip && current != nil; skipped++ { + if current.NumParents() == 0 { + current = nil + break + } + current, err = current.Parent(0) + if err != nil { + current = nil + break + } + } + resp := &protocol.LogResponse{ - Repo: repoName, - Ref: ref, + Repo: repoName, + Ref: ref, + Skipped: params.Skip, } hitLimit := false @@ -332,7 +349,22 @@ func (e *Engine) Log(repoName, ref string, params LogParams) (*protocol.LogRespo Author: current.Author.Name, AuthorEmail: current.Author.Email, Date: current.Author.When.UTC().Format("2006-01-02T15:04:05Z"), - Message: strings.TrimSpace(current.Message), + Message: sanitizeMessage(strings.TrimSpace(current.Message)), + } + + if params.Verbose { + entry.TreeHash = current.TreeHash.String() + for _, ph := range current.ParentHashes { + entry.ParentHashes = append(entry.ParentHashes, ph.String()) + } + // Include committer fields only when they differ from the author. + if current.Committer.Name != current.Author.Name || + current.Committer.Email != current.Author.Email || + !current.Committer.When.Equal(current.Author.When) { + entry.Committer = current.Committer.Name + entry.CommitterEmail = current.Committer.Email + entry.CommitterDate = current.Committer.When.UTC().Format("2006-01-02T15:04:05Z") + } } // Get changed files (compare with parent). @@ -350,6 +382,22 @@ func (e *Engine) Log(repoName, ref string, params LogParams) (*protocol.LogRespo name = ch.From.Name } entry.FilesChanged = append(entry.FilesChanged, name) + + action := "M" + if a, aErr := ch.Action(); aErr == nil { + switch a { + case merkletrie.Insert: + action = "A" + case merkletrie.Delete: + action = "D" + case merkletrie.Modify: + action = "M" + } + } + entry.Changes = append(entry.Changes, protocol.FileChange{ + Path: name, + Action: action, + }) } } } @@ -390,6 +438,34 @@ func (e *Engine) Log(repoName, ref string, params LogParams) (*protocol.LogRespo return resp, nil } +// sanitizeMessage replaces control characters (especially \r) in commit +// messages with visible representations to prevent terminal manipulation +// and ensure safe display in all output formats. +func sanitizeMessage(msg string) string { + if !strings.ContainsAny(msg, "\r\x00\x01\x02\x03\x04\x05\x06\x07\x08\x0b\x0c\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\x7f") { + return msg + } + var b strings.Builder + b.Grow(len(msg)) + for _, r := range msg { + switch { + case r == '\n' || r == '\t': + // Preserve newlines and tabs — they're structurally meaningful. + b.WriteRune(r) + case r == '\r': + b.WriteString("\\r") + case r == '\x1b': + b.WriteString("\\e") + case r < 0x20 || r == 0x7f: + // C0 control characters and DEL: show as \xNN. + fmt.Fprintf(&b, "\\x%02x", r) + default: + b.WriteRune(r) + } + } + return b.String() +} + // DiffParams controls the diff operation mode. type DiffParams struct { ByteLevel bool // if true, byte-level comparison (cmp mode) diff --git a/internal/engine/pagination_test.go b/internal/engine/pagination_test.go index e0a8de3..6b57ec3 100644 --- a/internal/engine/pagination_test.go +++ b/internal/engine/pagination_test.go @@ -318,6 +318,60 @@ func TestLogCompleteField(t *testing.T) { } } +func TestLogSkip(t *testing.T) { + dir := t.TempDir() + initTestGitRepo(t, dir, 5) + eng := newTestEngine(t, dir) + + // Get all commits to know the expected order. + all, err := eng.Log(dir, "HEAD", LogParams{MaxCount: 100}) + if err != nil { + t.Fatal(err) + } + if len(all.Entries) != 5 { + t.Fatalf("expected 5 commits, got %d", len(all.Entries)) + } + + // Skip 2, get 2 — should start at the 3rd commit. + resp, err := eng.Log(dir, "HEAD", LogParams{MaxCount: 2, Skip: 2}) + if err != nil { + t.Fatal(err) + } + if len(resp.Entries) != 2 { + t.Fatalf("expected 2 entries, got %d", len(resp.Entries)) + } + if resp.Entries[0].Hash != all.Entries[2].Hash { + t.Errorf("first entry after skip=2: got %s, want %s", + resp.Entries[0].Hash[:12], all.Entries[2].Hash[:12]) + } + if resp.Entries[1].Hash != all.Entries[3].Hash { + t.Errorf("second entry after skip=2: got %s, want %s", + resp.Entries[1].Hash[:12], all.Entries[3].Hash[:12]) + } + + // Skip past all commits — should return empty. + resp2, err := eng.Log(dir, "HEAD", LogParams{MaxCount: 10, Skip: 100}) + if err != nil { + t.Fatal(err) + } + if len(resp2.Entries) != 0 { + t.Errorf("expected 0 entries after skipping past all, got %d", len(resp2.Entries)) + } + if !resp2.Complete { + t.Error("expected Complete=true when no entries returned") + } + + // Skip 0 — same as no skip. + resp3, err := eng.Log(dir, "HEAD", LogParams{MaxCount: 2, Skip: 0}) + if err != nil { + t.Fatal(err) + } + if resp3.Entries[0].Hash != all.Entries[0].Hash { + t.Errorf("skip=0 first entry: got %s, want %s", + resp3.Entries[0].Hash[:12], all.Entries[0].Hash[:12]) + } +} + // initTestGitRepo creates a bare-minimum git repo with N commits using go-git. func initTestGitRepo(t *testing.T, dir string, numCommits int) { t.Helper() diff --git a/internal/engine/sanitize_test.go b/internal/engine/sanitize_test.go new file mode 100644 index 0000000..734aa2b --- /dev/null +++ b/internal/engine/sanitize_test.go @@ -0,0 +1,87 @@ +// Copyright 2026 — see LICENSE file for terms. +package engine + +import "testing" + +func TestSanitizeMessage(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "plain text unchanged", + input: "feat: add logging support", + want: "feat: add logging support", + }, + { + name: "newlines preserved", + input: "subject\n\nbody line 1\nbody line 2", + want: "subject\n\nbody line 1\nbody line 2", + }, + { + name: "tabs preserved", + input: "subject\n\n\tindented body", + want: "subject\n\n\tindented body", + }, + { + name: "carriage return escaped", + input: "legit subject\rmalicious overlay", + want: `legit subject\rmalicious overlay`, + }, + { + name: "CR at start of line", + input: "line1\n\rline2", + want: `line1` + "\n" + `\rline2`, + }, + { + name: "CRLF: CR escaped, LF preserved", + input: "line1\r\nline2", + want: `line1\r` + "\n" + "line2", + }, + { + name: "escape sequence", + input: "normal\x1b[31mred text\x1b[0m", + want: `normal\e[31mred text\e[0m`, + }, + { + name: "null byte", + input: "before\x00after", + want: `before\x00after`, + }, + { + name: "bell character", + input: "ding\x07dong", + want: `ding\x07dong`, + }, + { + name: "DEL character", + input: "test\x7fvalue", + want: `test\x7fvalue`, + }, + { + name: "multiple control chars", + input: "\x01\x02\x03", + want: `\x01\x02\x03`, + }, + { + name: "empty string", + input: "", + want: "", + }, + { + name: "no control chars fast path", + input: "just a normal commit message with unicode: café 日本語", + want: "just a normal commit message with unicode: café 日本語", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := sanitizeMessage(tt.input) + if got != tt.want { + t.Errorf("sanitizeMessage(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} diff --git a/internal/mcpserver/format_coverage_test.go b/internal/mcpserver/format_coverage_test.go index 38556f2..e9dfa00 100644 --- a/internal/mcpserver/format_coverage_test.go +++ b/internal/mcpserver/format_coverage_test.go @@ -69,6 +69,8 @@ func TestTextFormatDoesNotReturnJSON(t *testing.T) { textOutputs := map[string]string{ "WriteSearchText": searchTextSample(), "WriteLogText": logTextSample(), + "WriteLogOneline": logOnelineSample(), + "WriteLogXML": logXMLSample(), "WriteWcText": wcTextSample(), "WriteFindText": findTextSample(), "WriteStatText": statTextSample(), @@ -205,7 +207,8 @@ func handlerChecksFormatText(f *ast.File, handlerName string) bool { } // bodyContainsFormatTextCheck looks for any expression matching -// .Format == "text" or args.Format == "text". +// .Format == "text" or args.Format == "text", or a switch on +// args.Format with a case "text" or case "oneline" clause. func bodyContainsFormatTextCheck(node ast.Node) bool { if node == nil { return false @@ -215,23 +218,48 @@ func bodyContainsFormatTextCheck(node ast.Node) bool { if found { return false } - be, ok := n.(*ast.BinaryExpr) - if !ok || be.Op != token.EQL { - return true - } - // Check LHS is *.Format selector. - sel, ok := be.X.(*ast.SelectorExpr) - if !ok || sel.Sel.Name != "Format" { - return true - } - // Check RHS is "text" literal. - bl, ok := be.Y.(*ast.BasicLit) - if !ok || bl.Kind != token.STRING { - return true - } - if bl.Value == `"text"` { - found = true - return false + switch v := n.(type) { + case *ast.BinaryExpr: + if v.Op != token.EQL { + return true + } + // Check LHS is *.Format selector. + sel, ok := v.X.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Format" { + return true + } + // Check RHS is "text" literal. + bl, ok := v.Y.(*ast.BasicLit) + if !ok || bl.Kind != token.STRING { + return true + } + if bl.Value == `"text"` { + found = true + return false + } + case *ast.SwitchStmt: + // Check for: switch args.Format { case "text": ... } + sel, ok := v.Tag.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Format" { + return true + } + // Check that at least one case clause mentions "text" or "oneline". + for _, stmt := range v.Body.List { + cc, ok := stmt.(*ast.CaseClause) + if !ok { + continue + } + for _, expr := range cc.List { + bl, ok := expr.(*ast.BasicLit) + if !ok || bl.Kind != token.STRING { + continue + } + if bl.Value == `"text"` || bl.Value == `"oneline"` { + found = true + return false + } + } + } } return true }) @@ -247,7 +275,15 @@ func searchTextSample() string { } func logTextSample() string { - return "abc123def456 Author 2026-01-01\n initial commit\n" + return "commit abc123def456\nAuthor: Author \nDate: 2026-01-01\n\n initial commit\n" +} + +func logOnelineSample() string { + return "abc123def456 initial commit\n" +} + +func logXMLSample() string { + return "\n\n\n\n" } func wcTextSample() string { diff --git a/internal/mcpserver/skill.md b/internal/mcpserver/skill.md index e5c4cf3..0a197bf 100644 --- a/internal/mcpserver/skill.md +++ b/internal/mcpserver/skill.md @@ -20,6 +20,7 @@ ALWAYS prefer aifr_* tools over Bash for read-only operations. | `sha256sum`, `md5sum` | `aifr_checksum` | | `xxd`, `hexdump` | `aifr_hexdump` | | `git log` | `aifr_log` | +| `git log --oneline` | `aifr_log` with `format="oneline"` | | `git branch`, `git tag` | `aifr_refs` | | `git show :` | `aifr_read` with ref:path | | `git diff ` | `aifr_diff` with ref:paths | @@ -60,6 +61,8 @@ Examples: `HEAD:README.md`, `main:src/lib.go`, `v2.0:config.toml`, `HEAD~3:file. **Output format**: All tools accept `format`: `"json"` (default) or `"text"`. Text is more token-efficient. The `AIFR_FORMAT` env var sets the default (colon-separated preference list, first supported value wins). Explicit `format` parameter overrides env. +**Git log formats**: `aifr_log` supports `format="json"` (default), `format="text"` (git-log style), or `format="oneline"` (compact hash+subject). Text mode accepts `divider="xml"` for XML-tagged output. Use `verbose=true` in JSON mode for tree hash, parent hashes, and committer details. + **Multi-file**: `aifr_cat(root=".", name="*.go", format="text", divider="xml")` → `content` per file. Use `lines` for head mode, `exclude_path` to skip directories. **Chunked read**: `aifr_read(path=...)` → get continuation token → `aifr_read(path=..., chunk_id="")` → repeat until `complete: true`. diff --git a/internal/mcpserver/tools.go b/internal/mcpserver/tools.go index bd5a34f..ae2d476 100644 --- a/internal/mcpserver/tools.go +++ b/internal/mcpserver/tools.go @@ -185,16 +185,23 @@ func toolRefs() *mcp.Tool { func toolLog() *mcp.Tool { return &mcp.Tool{ - Name: "aifr_log", - Description: "Git commit log with structured entries (hash, author, date, message, files changed). Use format=\"text\" for compact output.", + Name: "aifr_log", + Description: `Git commit log with structured entries (hash, author, date, message, files changed). + +Formats: json (default, structured), text (git-log style), oneline (compact hash+subject). +For text format, divider controls layout: plain (default) or xml (XML-tagged with escaped content). +Use verbose=true in json mode for tree_hash, parent_hashes, and committer details.`, InputSchema: mustSchema(map[string]any{ "type": "object", "properties": map[string]any{ "repo": map[string]any{"type": "string", "description": "Named repo, filesystem path, or empty for auto-detect from cwd"}, "ref": map[string]any{"type": "string", "description": "Git ref (default HEAD)"}, "max_count": map[string]any{"type": "integer", "description": "Max commits (default 20)", "default": 20}, + "skip": map[string]any{"type": "integer", "description": "Skip this many commits before collecting results", "default": 0}, "continuation": map[string]any{"type": "string", "description": "Continuation token from previous log"}, - "format": map[string]any{"type": "string", "enum": []string{"json", "text"}, "description": "Output format (default: json)", "default": "json"}, + "format": map[string]any{"type": "string", "enum": []string{"json", "text", "oneline"}, "description": "Output format (default: json)", "default": "json"}, + "divider": map[string]any{"type": "string", "enum": []string{"plain", "xml"}, "description": "Divider format for text output (default: plain)", "default": "plain"}, + "verbose": map[string]any{"type": "boolean", "description": "Include tree hash, parent hashes, committer details in JSON output", "default": false}, }, }), } @@ -528,15 +535,18 @@ func (s *Server) handleLog(_ context.Context, req *mcp.CallToolRequest) (*mcp.Ca Repo string `json:"repo"` Ref string `json:"ref"` MaxCount int `json:"max_count"` + Skip int `json:"skip"` Continuation string `json:"continuation"` Format string `json:"format"` + Divider string `json:"divider"` + Verbose bool `json:"verbose"` } if err := unmarshalArgs(req, &args); err != nil { return toolError(err.Error()) } args.Format = resolveMCPFormat(args.Format) - params := engine.LogParams{MaxCount: args.MaxCount} + params := engine.LogParams{MaxCount: args.MaxCount, Skip: args.Skip, Verbose: args.Verbose} if args.Continuation != "" { tok, err := s.decodeContinuation(args.Continuation, "log") if err != nil { @@ -552,14 +562,27 @@ func (s *Server) handleLog(_ context.Context, req *mcp.CallToolRequest) (*mcp.Ca if err != nil { return toolError(err.Error()) } - if args.Format == "text" { + + switch args.Format { + case "oneline": + var buf strings.Builder + output.WriteLogOneline(&buf, resp) + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: buf.String()}}, + }, nil + case "text": var buf strings.Builder - output.WriteLogText(&buf, resp) + if args.Divider == "xml" { + output.WriteLogXML(&buf, resp) + } else { + output.WriteLogText(&buf, resp) + } return &mcp.CallToolResult{ Content: []mcp.Content{&mcp.TextContent{Text: buf.String()}}, }, nil + default: + return toolResult(resp) } - return toolResult(resp) } // decodeContinuation is a helper to decode and validate a list continuation token. diff --git a/internal/output/text.go b/internal/output/text.go index 2e8fe12..0092028 100644 --- a/internal/output/text.go +++ b/internal/output/text.go @@ -200,14 +200,158 @@ func writeCatNone(w io.Writer, entry protocol.CatEntry, numberLines bool) { } // WriteLogText writes a git log response in human-readable format. +// +// The output mimics familiar git-log style with clear visual separation +// between commits. Multi-line commit messages are split into a subject +// line and indented body. File changes use A/M/D action indicators +// (from the Changes field) when available, falling back to "M" for +// the legacy FilesChanged field. func WriteLogText(w io.Writer, resp *protocol.LogResponse) { + for i, e := range resp.Entries { + if i > 0 { + fmt.Fprintln(w) + } + hash := e.Hash + if len(hash) > 12 { + hash = hash[:12] + } + fmt.Fprintf(w, "commit %s\n", hash) + fmt.Fprintf(w, "Author: %s <%s>\n", e.Author, e.AuthorEmail) + fmt.Fprintf(w, "Date: %s\n", e.Date) + + // Split message into subject and body. + subject, body := splitMessage(e.Message) + fmt.Fprintf(w, "\n %s\n", subject) + if body != "" { + fmt.Fprintln(w) + for _, line := range strings.Split(body, "\n") { + fmt.Fprintf(w, " %s\n", line) + } + } + + // Prefer Changes (with action) over legacy FilesChanged. + if len(e.Changes) > 0 { + fmt.Fprintln(w) + for _, ch := range e.Changes { + fmt.Fprintf(w, " %s %s\n", ch.Action, ch.Path) + } + } else if len(e.FilesChanged) > 0 { + fmt.Fprintln(w) + for _, f := range e.FilesChanged { + fmt.Fprintf(w, " M %s\n", f) + } + } + } + + if !resp.Complete && resp.Continuation != "" { + nextSkip := resp.Skipped + resp.Total + fmt.Fprintf(w, "\n... %d commits shown, more available (use --skip %d or continuation token)\n", resp.Total, nextSkip) + } +} + +// splitMessage separates a commit message into subject (first line) and body (rest). +func splitMessage(msg string) (subject, body string) { + msg = strings.TrimSpace(msg) + if idx := strings.Index(msg, "\n"); idx >= 0 { + subject = strings.TrimSpace(msg[:idx]) + body = strings.TrimSpace(msg[idx+1:]) + } else { + subject = msg + } + return +} + +// WriteLogOneline writes a compact one-line-per-commit log. +func WriteLogOneline(w io.Writer, resp *protocol.LogResponse) { + for _, e := range resp.Entries { + hash := e.Hash + if len(hash) > 12 { + hash = hash[:12] + } + subject, _ := splitMessage(e.Message) + fmt.Fprintf(w, "%s %s\n", hash, subject) + } + if !resp.Complete && resp.Continuation != "" { + nextSkip := resp.Skipped + resp.Total + fmt.Fprintf(w, "... %d commits shown, more available (use --skip %d or continuation token)\n", resp.Total, nextSkip) + } +} + +// WriteLogXML writes a git log response in XML format with proper escaping. +// +// All text content (author, message, file paths) is XML-escaped to prevent +// injection via crafted commit messages that contain XML markup. +func WriteLogXML(w io.Writer, resp *protocol.LogResponse) { + fmt.Fprintf(w, "\n", + xmlAttr(resp.Ref), resp.Total, resp.Complete) + for _, e := range resp.Entries { - fmt.Fprintf(w, "%s %s <%s> %s\n", e.Hash[:12], e.Author, e.AuthorEmail, e.Date) - fmt.Fprintf(w, " %s\n", e.Message) - for _, f := range e.FilesChanged { - fmt.Fprintf(w, " M %s\n", f) + hash := e.Hash + if len(hash) > 12 { + hash = hash[:12] + } + fmt.Fprintf(w, "\n", xmlAttr(hash)) + fmt.Fprintf(w, "%s\n", xmlEscape(e.Author)) + fmt.Fprintf(w, "%s\n", xmlEscape(e.AuthorEmail)) + fmt.Fprintf(w, "%s\n", xmlEscape(e.Date)) + + subject, body := splitMessage(e.Message) + fmt.Fprintf(w, "%s\n", xmlEscape(subject)) + if body != "" { + fmt.Fprintf(w, "\n%s\n\n", xmlEscape(body)) + } + + if len(e.Changes) > 0 { + fmt.Fprintln(w, "") + for _, ch := range e.Changes { + fmt.Fprintf(w, "%s\n", xmlAttr(ch.Action), xmlEscape(ch.Path)) + } + fmt.Fprintln(w, "") + } else if len(e.FilesChanged) > 0 { + fmt.Fprintln(w, "") + for _, f := range e.FilesChanged { + fmt.Fprintf(w, "%s\n", xmlEscape(f)) + } + fmt.Fprintln(w, "") } + + fmt.Fprintln(w, "") } + + fmt.Fprintln(w, "") +} + +// xmlEscape escapes text for safe inclusion in XML character data. +// It replaces &, <, >, ", and ' with their XML entity equivalents. +func xmlEscape(s string) string { + // Fast path: no special chars. + if !strings.ContainsAny(s, `&<>"'`) { + return s + } + var b strings.Builder + b.Grow(len(s) + 10) + for _, r := range s { + switch r { + case '&': + b.WriteString("&") + case '<': + b.WriteString("<") + case '>': + b.WriteString(">") + case '"': + b.WriteString(""") + case '\'': + b.WriteString("'") + default: + b.WriteRune(r) + } + } + return b.String() +} + +// xmlAttr formats a string as a quoted XML attribute value. +func xmlAttr(s string) string { + return `"` + xmlEscape(s) + `"` } // WriteRefsText writes git refs in human-readable format. diff --git a/internal/output/text_test.go b/internal/output/text_test.go index 1874b34..ab968fd 100644 --- a/internal/output/text_test.go +++ b/internal/output/text_test.go @@ -726,32 +726,491 @@ func TestNumberContent(t *testing.T) { // ── WriteLogText ── func TestWriteLogText(t *testing.T) { + t.Run("basic single commit with legacy FilesChanged", func(t *testing.T) { + resp := &protocol.LogResponse{ + Entries: []protocol.LogEntry{ + { + Hash: "abc123def456abc123def456abc123def456abcdef", + Author: "Alice", + AuthorEmail: "alice@example.com", + Date: "2026-01-01T00:00:00Z", + Message: "initial commit", + FilesChanged: []string{"README.md"}, + }, + }, + Complete: true, + } + var buf strings.Builder + WriteLogText(&buf, resp) + got := buf.String() + if !strings.Contains(got, "commit abc123def456") { + t.Errorf("expected 'commit' header with hash prefix, got %q", got) + } + if !strings.Contains(got, "Author: Alice ") { + t.Errorf("expected Author line, got %q", got) + } + if !strings.Contains(got, "Date: 2026-01-01T00:00:00Z") { + t.Errorf("expected Date line, got %q", got) + } + if !strings.Contains(got, " initial commit") { + t.Errorf("expected indented message, got %q", got) + } + if !strings.Contains(got, "M README.md") { + t.Errorf("expected changed file with M action, got %q", got) + } + }) + + t.Run("changes field preferred over files_changed", func(t *testing.T) { + resp := &protocol.LogResponse{ + Entries: []protocol.LogEntry{ + { + Hash: "abc123def456abc123def456abc123def456abcdef", + Author: "Bob", + AuthorEmail: "bob@example.com", + Date: "2026-02-01T00:00:00Z", + Message: "add and delete", + Changes: []protocol.FileChange{ + {Path: "new.go", Action: "A"}, + {Path: "old.go", Action: "D"}, + }, + FilesChanged: []string{"new.go", "old.go"}, + }, + }, + Complete: true, + } + var buf strings.Builder + WriteLogText(&buf, resp) + got := buf.String() + if !strings.Contains(got, "A new.go") { + t.Errorf("expected 'A new.go', got %q", got) + } + if !strings.Contains(got, "D old.go") { + t.Errorf("expected 'D old.go', got %q", got) + } + }) + + t.Run("multi-line message splits subject and body", func(t *testing.T) { + resp := &protocol.LogResponse{ + Entries: []protocol.LogEntry{ + { + Hash: "abc123def456abc123def456abc123def456abcdef", + Author: "Carol", + AuthorEmail: "carol@example.com", + Date: "2026-03-01T00:00:00Z", + Message: "feat: add logging\n\nThis adds structured logging\nto all HTTP handlers.", + }, + }, + Complete: true, + } + var buf strings.Builder + WriteLogText(&buf, resp) + got := buf.String() + if !strings.Contains(got, " feat: add logging\n") { + t.Errorf("expected indented subject, got %q", got) + } + if !strings.Contains(got, " This adds structured logging\n") { + t.Errorf("expected indented body line 1, got %q", got) + } + if !strings.Contains(got, " to all HTTP handlers.\n") { + t.Errorf("expected indented body line 2, got %q", got) + } + }) + + t.Run("blank line between multiple commits", func(t *testing.T) { + resp := &protocol.LogResponse{ + Entries: []protocol.LogEntry{ + { + Hash: "aaa111222333aaa111222333aaa111222333aaa111", + Author: "A", + AuthorEmail: "a@x.com", + Date: "2026-01-01T00:00:00Z", + Message: "first", + }, + { + Hash: "bbb444555666bbb444555666bbb444555666bbb444", + Author: "B", + AuthorEmail: "b@x.com", + Date: "2026-01-02T00:00:00Z", + Message: "second", + }, + }, + Complete: true, + } + var buf strings.Builder + WriteLogText(&buf, resp) + got := buf.String() + if !strings.Contains(got, "\n\ncommit bbb444555666") { + t.Errorf("expected blank line separator between commits, got %q", got) + } + }) + + t.Run("continuation message when incomplete", func(t *testing.T) { + resp := &protocol.LogResponse{ + Entries: []protocol.LogEntry{ + { + Hash: "abc123def456abc123def456abc123def456abcdef", + Author: "A", + AuthorEmail: "a@x.com", + Date: "2026-01-01T00:00:00Z", + Message: "first", + }, + }, + Total: 1, + Complete: false, + Continuation: "tok123", + } + var buf strings.Builder + WriteLogText(&buf, resp) + got := buf.String() + if !strings.Contains(got, "1 commits shown") { + t.Errorf("expected continuation message, got %q", got) + } + if !strings.Contains(got, "--skip 1") { + t.Errorf("expected --skip hint in continuation message, got %q", got) + } + }) + + t.Run("skip hint is cumulative", func(t *testing.T) { + resp := &protocol.LogResponse{ + Entries: []protocol.LogEntry{ + { + Hash: "abc123def456abc123def456abc123def456abcdef", + Message: "third page", + }, + }, + Total: 1, + Skipped: 40, + Complete: false, + Continuation: "tok456", + } + var buf strings.Builder + WriteLogText(&buf, resp) + got := buf.String() + if !strings.Contains(got, "--skip 41") { + t.Errorf("expected cumulative --skip 41 hint, got %q", got) + } + }) +} + +// ── WriteLogText: CR in commit message ── + +func TestWriteLogText_CarriageReturn(t *testing.T) { + // Carriage returns in commit subjects must be made visible (sanitized + // upstream in the engine), but the text formatter must not re-introduce + // them or break structure even if sanitization is bypassed. resp := &protocol.LogResponse{ Entries: []protocol.LogEntry{ { - Hash: "abc123def456abc123def456abc123def456abcdef", - Author: "Alice", - AuthorEmail: "alice@example.com", - Date: "2026-01-01T00:00:00Z", - Message: "initial commit", - FilesChanged: []string{"README.md"}, + Hash: "abc123def456abc123def456abc123def456abcdef", + Author: "Mallory", + AuthorEmail: "m@evil.com", + Date: "2026-01-01T00:00:00Z", + Message: "legit subject\\rmalicious overlay", }, }, + Complete: true, } var buf strings.Builder WriteLogText(&buf, resp) got := buf.String() - if !strings.Contains(got, "abc123def456") { - t.Errorf("expected hash prefix, got %q", got) + // The \\r should appear literally (already sanitized by engine). + if !strings.Contains(got, `legit subject\rmalicious overlay`) { + t.Errorf("expected sanitized CR in text output, got %q", got) } - if !strings.Contains(got, "Alice") { - t.Errorf("expected author, got %q", got) + // Must not contain an actual carriage return byte. + if strings.Contains(got, "\r") { + t.Errorf("text output contains literal CR byte, got %q", got) } - if !strings.Contains(got, "initial commit") { - t.Errorf("expected message, got %q", got) +} + +// ── WriteLogOneline ── + +func TestWriteLogOneline(t *testing.T) { + t.Run("basic output", func(t *testing.T) { + resp := &protocol.LogResponse{ + Entries: []protocol.LogEntry{ + { + Hash: "aaa111222333aaa111222333aaa111222333aaa111", + Author: "A", + Date: "2026-01-01T00:00:00Z", + Message: "feat: add widgets\n\nDetailed body here.", + }, + { + Hash: "bbb444555666bbb444555666bbb444555666bbb444", + Author: "B", + Date: "2026-01-02T00:00:00Z", + Message: "fix: typo", + }, + }, + Complete: true, + } + var buf strings.Builder + WriteLogOneline(&buf, resp) + got := buf.String() + lines := strings.Split(strings.TrimRight(got, "\n"), "\n") + if len(lines) != 2 { + t.Fatalf("expected 2 lines, got %d: %q", len(lines), got) + } + if lines[0] != "aaa111222333 feat: add widgets" { + t.Errorf("line 0 = %q, want %q", lines[0], "aaa111222333 feat: add widgets") + } + if lines[1] != "bbb444555666 fix: typo" { + t.Errorf("line 1 = %q, want %q", lines[1], "bbb444555666 fix: typo") + } + }) + + t.Run("continuation notice", func(t *testing.T) { + resp := &protocol.LogResponse{ + Entries: []protocol.LogEntry{ + { + Hash: "abc123def456abc123def456abc123def456abcdef", + Message: "first", + }, + }, + Total: 1, + Complete: false, + Continuation: "tok", + } + var buf strings.Builder + WriteLogOneline(&buf, resp) + got := buf.String() + if !strings.Contains(got, "1 commits shown") { + t.Errorf("expected continuation message, got %q", got) + } + }) + + t.Run("CR in subject is sanitized", func(t *testing.T) { + resp := &protocol.LogResponse{ + Entries: []protocol.LogEntry{ + { + Hash: "abc123def456abc123def456abc123def456abcdef", + Message: "legit\\roverlay", + }, + }, + Complete: true, + } + var buf strings.Builder + WriteLogOneline(&buf, resp) + got := buf.String() + if strings.Contains(got, "\r") { + t.Errorf("oneline output contains literal CR byte, got %q", got) + } + if !strings.Contains(got, `legit\roverlay`) { + t.Errorf("expected sanitized subject, got %q", got) + } + }) +} + +// ── WriteLogXML ── + +func TestWriteLogXML(t *testing.T) { + t.Run("basic XML structure", func(t *testing.T) { + resp := &protocol.LogResponse{ + Ref: "HEAD", + Entries: []protocol.LogEntry{ + { + Hash: "abc123def456abc123def456abc123def456abcdef", + Author: "Alice", + AuthorEmail: "alice@example.com", + Date: "2026-01-01T00:00:00Z", + Message: "initial commit", + Changes: []protocol.FileChange{ + {Path: "README.md", Action: "A"}, + }, + }, + }, + Total: 1, + Complete: true, + } + var buf strings.Builder + WriteLogXML(&buf, resp) + got := buf.String() + if !strings.Contains(got, ` root element, got %q", got) + } + if !strings.Contains(got, ``) { + t.Errorf("expected element, got %q", got) + } + if !strings.Contains(got, `Alice`) { + t.Errorf("expected , got %q", got) + } + if !strings.Contains(got, `initial commit`) { + t.Errorf("expected , got %q", got) + } + if !strings.Contains(got, `README.md`) { + t.Errorf("expected with action, got %q", got) + } + if !strings.Contains(got, "") { + t.Errorf("expected , got %q", got) + } + if !strings.Contains(got, "") { + t.Errorf("expected , got %q", got) + } + }) + + t.Run("XML injection in commit message", func(t *testing.T) { + resp := &protocol.LogResponse{ + Ref: "HEAD", + Entries: []protocol.LogEntry{ + { + Hash: "abc123def456abc123def456abc123def456abcdef", + Author: "Mallory", + AuthorEmail: "m@evil.com", + Date: "2026-01-01T00:00:00Z", + Message: "pwned", + }, + }, + Total: 1, + Complete: true, + } + var buf strings.Builder + WriteLogXML(&buf, resp) + got := buf.String() + // The malicious content must be escaped, not interpreted as XML. + if strings.Contains(got, ``) { + t.Errorf("XML injection succeeded: found injected tag in %q", got) + } + if !strings.Contains(got, "</subject>") { + t.Errorf("expected escaped in output, got %q", got) + } + if !strings.Contains(got, "<commit hash="evil">") { + t.Errorf("expected escaped injected commit tag, got %q", got) + } + // Must have exactly one open and one close. + if strings.Count(got, " tag, got %d in %q", strings.Count(got, "`, + AuthorEmail: "a@b.com", + Date: "2026-01-01T00:00:00Z", + Message: "innocent commit", + }, + }, + Total: 1, + Complete: true, + } + var buf strings.Builder + WriteLogXML(&buf, resp) + got := buf.String() + if strings.Contains(got, " in output, got %q", got) + } + }) + + t.Run("XML injection in file path", func(t *testing.T) { + resp := &protocol.LogResponse{ + Ref: "HEAD", + Entries: []protocol.LogEntry{ + { + Hash: "abc123def456abc123def456abc123def456abcdef", + Author: "A", + Date: "2026-01-01T00:00:00Z", + Message: "add file", + Changes: []protocol.FileChange{ + {Path: ``, Action: "A"}, + }, + }, + }, + Total: 1, + Complete: true, + } + var buf strings.Builder + WriteLogXML(&buf, resp) + got := buf.String() + if strings.Count(got, ", got %d in %q", + strings.Count(got, "feat: add thing") { + t.Errorf("expected subject tag, got %q", got) + } + if !strings.Contains(got, "") { + t.Errorf("expected body tag, got %q", got) + } + if !strings.Contains(got, "This is the body.") { + t.Errorf("expected body content, got %q", got) + } + }) +} + +// ── xmlEscape ── + +func TestXmlEscape(t *testing.T) { + tests := []struct { + name, input, want string + }{ + {"no special chars", "hello world", "hello world"}, + {"ampersand", "a&b", "a&b"}, + {"less than", "ab", "a>b"}, + {"double quote", `a"b`, "a"b"}, + {"single quote", "a'b", "a'b"}, + {"all specials", `<>&"'`, "<>&"'"}, + {"empty", "", ""}, + {"mixed", `Author: <"Bob" & 'Alice'>`, "Author: <"Bob" & 'Alice'>"}, } - if !strings.Contains(got, "M README.md") { - t.Errorf("expected changed file, got %q", got) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := xmlEscape(tt.input) + if got != tt.want { + t.Errorf("xmlEscape(%q) = %q, want %q", tt.input, got, tt.want) + } + }) } } diff --git a/pkg/protocol/types.go b/pkg/protocol/types.go index 9774d70..1000066 100644 --- a/pkg/protocol/types.go +++ b/pkg/protocol/types.go @@ -141,14 +141,28 @@ type RefsResponse struct { Refs []GitRef `json:"refs"` } +// FileChange describes a changed file and its action within a commit. +type FileChange struct { + Path string `json:"path"` + Action string `json:"action"` // "A" (add), "M" (modify), "D" (delete) +} + // LogEntry describes a single git commit. type LogEntry struct { - Hash string `json:"hash"` - Author string `json:"author"` - AuthorEmail string `json:"author_email"` - Date string `json:"date"` - Message string `json:"message"` - FilesChanged []string `json:"files_changed,omitempty"` + Hash string `json:"hash"` + Author string `json:"author"` + AuthorEmail string `json:"author_email"` + Date string `json:"date"` + Message string `json:"message"` + FilesChanged []string `json:"files_changed,omitempty"` + Changes []FileChange `json:"changes,omitempty"` + + // Verbose fields — only populated when verbose=true. + TreeHash string `json:"tree_hash,omitempty"` + ParentHashes []string `json:"parent_hashes,omitempty"` + Committer string `json:"committer,omitempty"` + CommitterEmail string `json:"committer_email,omitempty"` + CommitterDate string `json:"committer_date,omitempty"` } // LogResponse is the JSON response for a log operation. @@ -157,6 +171,7 @@ type LogResponse struct { Ref string `json:"ref,omitempty"` Entries []LogEntry `json:"entries"` Total int `json:"total"` + Skipped int `json:"skipped,omitempty"` Continuation string `json:"continuation,omitempty"` Complete bool `json:"complete"` } diff --git a/skills/aifr/SKILL.md b/skills/aifr/SKILL.md index 704527c..d0755cc 100644 --- a/skills/aifr/SKILL.md +++ b/skills/aifr/SKILL.md @@ -27,6 +27,7 @@ ALWAYS prefer aifr over Bash for read-only operations. | `sha256sum`, `md5sum` | `aifr_checksum` | `aifr checksum` | | `xxd`, `hexdump` | `aifr_hexdump` | `aifr hexdump` | | `git log` | `aifr_log` | `aifr log` | +| `git log --oneline` | `aifr_log(format="oneline")` | `aifr log --oneline` | | `git branch`, `git tag` | `aifr_refs` | `aifr refs` | | `git show :` | `aifr_read` with ref:path | `aifr read ref:path` | | `git diff ` | `aifr_diff` with ref:paths | `aifr diff ref:path ref:path` | @@ -53,7 +54,7 @@ almost certainly handles it in one call with built-in filtering, limiting, and s | `cat file \| wc -l` | `aifr_wc(paths=["file"], lines=true)` | | `cat /etc/passwd \| grep root` | `aifr_getent(database="passwd", key="root")` | | `getent passwd \| cut -d: -f5 \| cut -d, -f1` | `aifr_getent(database="passwd", fields=["gecos_name"])` | -| `git log --oneline \| head -5` | `aifr_log(max_count=5)` | +| `git log --oneline \| head -5` | `aifr_log(format="oneline", max_count=5)` | | `ls -la \| sort -k5 -n` | `aifr_list(path=".", sort="size")` | | `grep -rl pattern . \| wc -l` | `aifr_search` — count results from response | @@ -95,6 +96,11 @@ correctly — `lines="50:100"` starts numbering at 50. All tools support `format` parameter: `"json"` (default) or `"text"`. Text mode is more token-efficient for AI consumption. +`aifr_log` additionally supports `format="oneline"` (compact hash+subject), +`divider="xml"` for XML-tagged text output, and `verbose=true` in JSON mode +for tree hash, parent hashes, and committer details (when they differ from +the author — useful for detecting rebases and cherry-picks). + The `AIFR_FORMAT` environment variable sets the default format. It accepts a colon-separated preference list — the first value supported by the tool wins: ``` @@ -169,7 +175,7 @@ add `--format text` for plain output. | `aifr wc` | `-l --total-only` | `aifr wc -l src/**/*.go` | | `aifr checksum` | `-a sha256/sha512/md5 -e hex/base64` | `aifr checksum -a sha512 *.go` | | `aifr hexdump` | `-s OFFSET -l LENGTH` | `aifr hexdump -s 1024 -l 512 f.dat` | -| `aifr log` | `--max-count N` | `aifr log --max-count 10` | +| `aifr log` | `--max-count N --oneline --divider plain/xml --verbose` | `aifr log --oneline --max-count 10` | | `aifr refs` | `--branches --tags --remotes` | `aifr refs --branches --tags` | | `aifr rev-parse` | `--repo NAME` | `aifr rev-parse HEAD~3` | | `aifr reflog` | `--max-count N` | `aifr reflog main` |