Skip to content

Improve log output formats: oneline, XML, verbose, safety#7

Merged
philpennock merged 5 commits intomainfrom
claude/improve-log-formatting-jVni3
Apr 3, 2026
Merged

Improve log output formats: oneline, XML, verbose, safety#7
philpennock merged 5 commits intomainfrom
claude/improve-log-formatting-jVni3

Conversation

@philpennock
Copy link
Copy Markdown
Member

Summary

Reworks aifr log output formatting in response to #6, adding multiple output modes and hardening against malicious commit content.

New output formats:

  • format=oneline — compact hash subject per line (CLI: --oneline), token-efficient for orientation queries
  • divider=xml — XML-tagged text output with <commit>, <subject>, <body>, <files> structure
  • format=text (improved) — git-log style with commit/Author:/Date: headers, subject/body splitting, blank-line separators

Verbose JSON mode (--verbose / verbose=true):

  • tree_hash and parent_hashes on every entry
  • committer, committer_email, committer_date only when they differ from author (flags rebases, cherry-picks, amends)

Structured file changes:

  • New changes JSON field with {path, action} objects using proper A/M/D indicators from go-git's merkletrie (instead of hardcoded "M")
  • Legacy files_changed preserved for backwards compatibility

Safety hardening:

  • sanitizeMessage() strips control characters from commit messages: CR→\r, ESC→\e, other C0→\xNN. Newlines and tabs preserved.
  • XML output uses xmlEscape() / xmlAttr() for all text content and attribute values, preventing injection via crafted commit messages, author names, or file paths.

Test plan

  • sanitize_test.go: 13 cases — CR, ESC, null, DEL, mixed control chars, unicode, fast-path
  • text_test.go: oneline format, XML structure, XML injection (via message, author, file path), CR in all formats, multi-line subject/body splitting, commit separators, continuation notices
  • format_coverage_test.go: updated AST check to recognize switch-based format dispatch; added oneline and XML samples
  • go test -race ./... passes all packages

Known issue (out of scope): aifr log . triggers access-control denial while aifr log (no arg) succeeds for the same repo — the "." path is treated as an explicit filesystem path rather than auto-detect. This affects all git subcommands (refs, reflog, etc.) and will be addressed in a follow-up.

https://claude.ai/code/session_016QBk2VFvP92AXspnSePeLz

claude added 2 commits April 3, 2026 09:28
Rework `--format=text` log output to mimic standard git-log style:
- "commit <hash>" header line for clear commit separation
- "Author:" and "Date:" labeled fields
- Subject/body message splitting with 4-space indentation
- Blank line separators between commits
- Proper A/M/D file change indicators via new `changes` JSON field
- Continuation notice when more commits are available

Add FileChange struct to protocol for typed change actions, populated
from go-git's merkletrie.Action (Insert/Delete/Modify). The legacy
`files_changed` field is preserved for backwards compatibility.

Relates to #6.

https://claude.ai/code/session_016QBk2VFvP92AXspnSePeLz
…ion for log

New log output options:
- format=oneline: compact hash+subject per line (CLI --oneline flag)
- divider=xml: XML-tagged text output with full escaping to prevent
  injection via crafted commit messages or file paths
- verbose=true: JSON mode includes tree_hash, parent_hashes, and
  committer/committer_email/committer_date (only when they differ
  from the author, to flag rebases/cherry-picks)

Safety:
- sanitizeMessage() in engine strips control characters (CR→\r,
  ESC→\e, other C0→\x##) from commit messages before they reach
  any formatter. Newlines and tabs are preserved.
- XML output uses xmlEscape() for all text content and xmlAttr()
  for attribute values, preventing event injection.

Tests added:
- sanitize_test.go: 13 cases covering CR, ESC, null, DEL, mixed
  control chars, unicode, and fast-path (no control chars)
- text_test.go: oneline format, XML structure, XML injection via
  commit message/author/file path, CR in all formats, multi-line
  message subject/body splitting
- format_coverage_test.go: updated AST check to recognize switch
  statements (not just == comparisons) for format dispatch

Updated: README.md, skill.md, SKILL.md with new options.

Relates to #6.

https://claude.ai/code/session_016QBk2VFvP92AXspnSePeLz
@philpennock philpennock force-pushed the claude/improve-log-formatting-jVni3 branch from 589d071 to 746630a Compare April 3, 2026 09:28
cmd/aifr/main.go Outdated
output.WriteLogOneline(os.Stdout, resp)
return
}
// Non-log types fall through to JSON.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct, when the format was given as oneline:text ? Shouldn't we honor that precedence list?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. In practice PersistentPreRunE calls ResolveFormat with per-command supported lists (cliSupportedFormats), so AIFR_FORMAT=oneline:text would skip oneline for non-log commands and resolve to text before writeOutput is ever called. Similarly resolveMCPFormat hardcodes ["json","text"].

But you're right that the silent JSON fallback was a code smell — if that path were ever reached, it'd be a bug that we'd want to notice, not silently degrade.

Fixed in cf6f99e: moved oneline dispatch into the *protocol.LogResponse case of the type switch, alongside the divider logic. No more top-level guard or fallback path.


Generated by Claude Code

claude added 3 commits April 3, 2026 09:38
…back

Addresses review feedback: the previous top-level oneline guard would
silently fall through to JSON for non-log types. Move oneline handling
into the LogResponse switch case alongside divider dispatch, so the
format is only checked where it's meaningful. PersistentPreRunE already
validates format per-command, but this removes the misleading fallback.

https://claude.ai/code/session_016QBk2VFvP92AXspnSePeLz
Add skip parameter to engine LogParams, CLI (--skip N), and MCP
(skip: N) so users can page through log results without needing
continuation tokens. Composes naturally with --max-count:

  aifr log --oneline --max-count 20
  aifr log --oneline --max-count 20 --skip 20

Update text/oneline continuation messages to suggest --skip as
an alternative to continuation tokens, since CLI users have no
way to pass tokens back.

https://claude.ai/code/session_016QBk2VFvP92AXspnSePeLz
Add Skipped field to LogResponse so text/oneline formatters can
compute the cumulative next-page offset. Previously the hint always
showed the current page's count; now it shows Skipped + Total,
giving a correct --skip value for the next page.

https://claude.ai/code/session_016QBk2VFvP92AXspnSePeLz
Copy link
Copy Markdown
Member Author

@philpennock philpennock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@philpennock philpennock merged commit 4f9e31e into main Apr 3, 2026
6 checks passed
@philpennock philpennock deleted the claude/improve-log-formatting-jVni3 branch April 3, 2026 09:56
@philpennock philpennock mentioned this pull request Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants