From 16a48ae1d9db23dd0591a10b3047663d44be3072 Mon Sep 17 00:00:00 2001 From: angelcodes95 Date: Wed, 4 Nov 2026 09:01:00 -0800 Subject: [PATCH 1/5] test: share one record() fixture instead of four copies check, format, merge-draft, and query tests each defined an identical record() helper. It now lives in tests/fixtures/record.ts. Agent-Context: {"why":"Done ahead of adding hash fields to AgentCommitRecord, which would otherwise have meant the same edit in four places, and four places to drift the next time the record shape changes.","touches":["cli/tests/fixtures/record.ts","cli/tests"]} --- cli/tests/check.test.ts | 13 +------------ cli/tests/fixtures/record.ts | 13 +++++++++++++ cli/tests/format.test.ts | 13 +------------ cli/tests/merge-draft.test.ts | 13 +------------ cli/tests/query.test.ts | 12 +----------- 5 files changed, 17 insertions(+), 47 deletions(-) create mode 100644 cli/tests/fixtures/record.ts diff --git a/cli/tests/check.test.ts b/cli/tests/check.test.ts index 7ecade1..e93c43f 100644 --- a/cli/tests/check.test.ts +++ b/cli/tests/check.test.ts @@ -1,17 +1,6 @@ import { describe, expect, it } from "vitest"; import { checkRecords } from "../src/check.js"; -import type { AgentCommitRecord } from "../src/types.js"; - -function record(overrides: Partial = {}): AgentCommitRecord { - return { - subject: "subject", - context: null, - malformed: false, - ambiguous: false, - misplaced: false, - ...overrides, - }; -} +import { record } from "./fixtures/record.js"; describe("checkRecords", () => { it("marks a commit with a valid object trailer as ok", () => { diff --git a/cli/tests/fixtures/record.ts b/cli/tests/fixtures/record.ts new file mode 100644 index 0000000..c872f2b --- /dev/null +++ b/cli/tests/fixtures/record.ts @@ -0,0 +1,13 @@ +import type { AgentCommitRecord } from "../../src/types.js"; + +/** Builds a synthetic AgentCommitRecord with neutral defaults, overriding only what a test cares about. */ +export function record(overrides: Partial = {}): AgentCommitRecord { + return { + subject: "subject", + context: null, + malformed: false, + ambiguous: false, + misplaced: false, + ...overrides, + }; +} diff --git a/cli/tests/format.test.ts b/cli/tests/format.test.ts index 5f99031..d915ba8 100644 --- a/cli/tests/format.test.ts +++ b/cli/tests/format.test.ts @@ -1,17 +1,6 @@ import { describe, expect, it } from "vitest"; import { formatJson, formatPretty, formatPrettyList, sanitizeForTerminal } from "../src/format.js"; -import type { AgentCommitRecord } from "../src/types.js"; - -function record(overrides: Partial = {}): AgentCommitRecord { - return { - subject: "subject", - context: null, - malformed: false, - ambiguous: false, - misplaced: false, - ...overrides, - }; -} +import { record } from "./fixtures/record.js"; describe("formatPretty", () => { it("prints just the subject when there's no trailer", () => { diff --git a/cli/tests/merge-draft.test.ts b/cli/tests/merge-draft.test.ts index 874cdb0..7a8aafb 100644 --- a/cli/tests/merge-draft.test.ts +++ b/cli/tests/merge-draft.test.ts @@ -1,20 +1,9 @@ import { afterEach, describe, expect, it } from "vitest"; import { buildMergeDraft, formatMergeDraft } from "../src/merge-draft.js"; import { parseLog, runGitLog } from "../src/parser.js"; -import type { AgentCommitRecord } from "../src/types.js"; +import { record } from "./fixtures/record.js"; import { TempRepo } from "./fixtures/temp-repo.js"; -function record(overrides: Partial = {}): AgentCommitRecord { - return { - subject: "subject", - context: null, - malformed: false, - ambiguous: false, - misplaced: false, - ...overrides, - }; -} - describe("buildMergeDraft", () => { it("unions touches and refs across commits, deduped", () => { const draft = buildMergeDraft([ diff --git a/cli/tests/query.test.ts b/cli/tests/query.test.ts index c1aa85d..fff5020 100644 --- a/cli/tests/query.test.ts +++ b/cli/tests/query.test.ts @@ -1,17 +1,7 @@ import { describe, expect, it } from "vitest"; import { queryRecords } from "../src/query.js"; import type { AgentCommitRecord } from "../src/types.js"; - -function record(overrides: Partial = {}): AgentCommitRecord { - return { - subject: "subject", - context: null, - malformed: false, - ambiguous: false, - misplaced: false, - ...overrides, - }; -} +import { record } from "./fixtures/record.js"; // No `type` key anywhere here: SPEC.md is explicit that `type` isn't a // field this format uses, so realistic fixtures shouldn't have one, and From 197fc67a67976b17902bd89279cc7705714e0016 Mon Sep 17 00:00:00 2001 From: angelcodes95 Date: Wed, 4 Nov 2026 09:02:10 -0800 Subject: [PATCH 2/5] fix(parser): stop an empty-message commit swallowing its neighbor Records were split on a "\0\0" boundary. A commit made with --allow-empty-message emits two adjacent empty fields, byte-identical to that boundary, so log merged it with the next commit and reported the remains as MISPLACED. The parser now reads a fixed count of NUL-terminated fields per commit and throws LogFormatError on output that isn't a whole number of records. Agent-Context: {"why":"Reproduced against real git first: two commits in, one bogus MISPLACED record out. Considered a different multi-byte boundary, rejected because NUL is the only byte git guarantees never appears in a message, so any boundary built from it can collide with empty fields; counting fields positionally is the only scheme with no collision case. Also unblocks adding a hash field, since a leading field would have collided the same way whenever a commit has no trailer.","touches":["cli/src/parser.ts","cli/tests/parser.test.ts"]} --- CHANGELOG.md | 9 +++ cli/src/parser.ts | 119 +++++++++++++++++++++++---------------- cli/tests/parser.test.ts | 43 ++++++++++++-- 3 files changed, 118 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 367f0aa..ac1dbfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,15 @@ version). ## [Unreleased] +### Fixed + +- A commit with an empty message (`git commit --allow-empty-message`) + no longer swallows the commit next to it. Records were split on a + double-NUL boundary, and an empty message produced two adjacent empty + fields that looked identical to it, so `log` dropped the neighboring + commit and misreported what remained as `MISPLACED`. The parser now + reads a fixed number of NUL-terminated fields per commit. + ## [0.1.0] - 2026-09-11 First published release. diff --git a/cli/src/parser.ts b/cli/src/parser.ts index aff176e..7eec3b4 100644 --- a/cli/src/parser.ts +++ b/cli/src/parser.ts @@ -2,39 +2,45 @@ import { execFileSync } from "node:child_process"; import type { AgentCommitRecord, ParseResult, ParseWarning } from "./types.js"; /** - * A single NUL byte, as a field separator between a commit's trailer - * extraction and its raw body. A NUL byte is the one thing guaranteed to - * never appear inside a git commit message, git itself won't let one - * through, so it can't collide with real content the way an earlier, - * printable-string version of this separator once did (a commit whose - * body happened to contain that literal string, for example one - * documenting this very format, silently split into phantom records; see - * SPEC.md, "Reading it back," for the incident this replaced). + * A single NUL byte, terminating every field of every record. A NUL byte + * is the one thing guaranteed to never appear inside a git commit message + * (git itself won't let one through), so it can't collide with real + * content the way an earlier, printable-string separator once did (a + * commit whose body happened to contain that literal string, for example + * one documenting this very format, silently split into phantom records; + * see SPEC.md, "Reading it back," for the incident this replaced). */ export const FIELD_SEP = "\0"; -/** Two NUL bytes in a row, as the boundary between one commit's record and the next. Unambiguous for the same reason FIELD_SEP is: content can never contain even one NUL, let alone two adjacent ones. */ -export const SENTINEL = "\0\0"; +/** + * Fields emitted per commit by LOG_FORMAT, in order: trailer extraction, + * raw body. Records are parsed + * positionally, LOG_FORMAT_FIELD_COUNT fields at a time, rather than split + * on a multi-byte record boundary: any field can legitimately be empty (no + * trailer, or a commit made with --allow-empty-message), and two adjacent + * empty-field terminators are byte-identical to a "\0\0" boundary, which + * is exactly how an empty-message commit used to swallow the record next + * to it. + */ +export const LOG_FORMAT_FIELD_COUNT = 2; /** * Internal to the CLI, deliberately richer than the plain-git example in * SPEC.md's "Reading it back" section. That example stays a minimal - * two-field command anyone can run with no tool; this format adds the raw - * commit body so the CLI can detect a trailer that's present in the - * message but sitting outside git's recognized trailer block (see the - * MISPLACED detection in parseLog below), which the plain two-field - * command cannot see at all. The subject is derived from the body's first - * line rather than fetched separately, one fewer field, one fewer thing - * that could disagree with itself. + * two-field command anyone can run with no tool; this format adds the + * raw commit body (so the CLI can detect a trailer that's present in the + * message but sitting outside git's recognized trailer block, see the + * MISPLACED detection in parseLog below). The subject is derived from the + * body's first line rather than fetched separately, one fewer field that + * could disagree with itself. * * Uses `%x00`, git's own escape syntax for emitting a NUL byte, not a * literal NUL character in this string: this format string is passed as a * command-line argument, and a real embedded NUL there is invalid at the * OS/argv level (Node rejects it outright). Git converts `%x00` into an * actual NUL byte only in its captured *output*, a different code path - * with no such restriction, that output byte is what FIELD_SEP/SENTINEL - * above are matching against in parseLog. Verified by inspecting git's - * raw output byte-for-byte before writing this. + * with no such restriction, that output byte is what FIELD_SEP above + * matches against in parseLog. */ -export const LOG_FORMAT = "%(trailers:key=Agent-Context,valueonly)%x00%B%x00%x00"; +export const LOG_FORMAT = "%(trailers:key=Agent-Context,valueonly)%x00%B%x00"; // Tolerates optional whitespace before the colon (git itself does: `Agent-Context : {...}` // is extracted as a trailer just as readily as `Agent-Context: {...}`, verified against real // git) and is case-insensitive to match git's own case-insensitive trailer key matching. @@ -118,7 +124,7 @@ export function checkGitVersion(cwd?: string): void { } } -/** Runs `git log` with the sentinel-delimited format from SPEC.md. Extra args are passed through (revision ranges, --since, path filters, etc). */ +/** Runs `git log` with LOG_FORMAT, the NUL-delimited format parseLog expects. Extra args are passed through (revision ranges, --since, path filters, etc). */ export function runGitLog(extraArgs: string[] = [], cwd?: string): string { try { return execFileSync("git", ["log", `--format=${LOG_FORMAT}`, ...extraArgs], { @@ -146,53 +152,72 @@ export function runGitLog(extraArgs: string[] = [], cwd?: string): string { } } +/** Raised when git log output doesn't have the shape LOG_FORMAT produces. Indicates a bug, not bad commit content. */ +export class LogFormatError extends Error { + constructor(fieldCount: number) { + super( + `unexpected git log output: ${fieldCount} fields is not a multiple of ${LOG_FORMAT_FIELD_COUNT}`, + ); + this.name = "LogFormatError"; + } +} + /** - * Parses raw `git log --format=` output into records. Each - * commit's record (between SENTINELs, two adjacent NULs) has two fields - * split on a single NUL: the trailer extraction (0, 1, or 2+ non-empty - * lines: 0 means no trailer, 1 means a normal trailer, 2+ means a - * duplicate, treated as ambiguous per SPEC.md rather than guessing which - * is authoritative), and the raw commit body, whose first line is the - * subject and whose full text is used only to detect a trailer that's - * present in the message but not in the position git recognizes as a - * trailer, exactly what GitHub's squash-merge prefill can silently - * produce (see SPEC.md, "Squash merges"). + * Parses raw `git log --format=` output into records. The + * output is split on NUL into a flat field list and consumed + * LOG_FORMAT_FIELD_COUNT fields per commit (see that constant for why + * positional parsing replaced a record boundary). Per commit: + * + * - the trailer extraction holds 0, 1, or 2+ non-empty lines: 0 means no + * trailer, 1 means a normal trailer, 2+ means a duplicate, treated as + * ambiguous per SPEC.md rather than guessing which is authoritative; + * - the raw body's first line is the subject, and its full text is used + * only to detect a trailer that's present in the message but not in the + * position git recognizes as a trailer, exactly what GitHub's + * squash-merge prefill can silently produce (see SPEC.md, "Squash + * merges"). */ export function parseLog(raw: string): ParseResult { const records: AgentCommitRecord[] = []; const warnings: ParseWarning[] = []; - const chunks = raw.split(SENTINEL); - - for (const chunk of chunks) { - const trimmed = chunk.replace(/^\n+/, "").replace(/\n+$/, ""); - if (trimmed === "") continue; + // Every field ends in FIELD_SEP, so the final element is whatever + // follows the last commit's last field: git's own record-terminating + // newline, or nothing. + const fields = raw.split(FIELD_SEP); + const trailing = fields.pop() ?? ""; + if (trailing.trim() !== "" || fields.length % LOG_FORMAT_FIELD_COUNT !== 0) { + throw new LogFormatError(fields.length); + } - const sepIndex = trimmed.indexOf(FIELD_SEP); - const trailerSection = sepIndex === -1 ? "" : trimmed.slice(0, sepIndex); - const rawBody = sepIndex === -1 ? trimmed : trimmed.slice(sepIndex + FIELD_SEP.length); + for (let i = 0; i < fields.length; i += LOG_FORMAT_FIELD_COUNT) { + // git separates records with a newline, which lands at the start of + // the next record's first field; trailer values never start with one. + const trailerSection = fields[i].replace(/^\n/, ""); + const rawBody = fields[i + 1].replace(/\n+$/, ""); const subject = rawBody.split("\n")[0] ?? ""; const trailerLines = trailerSection.split("\n").filter((line) => line.trim() !== ""); + const base = { subject }; if (trailerLines.length === 0) { const misplaced = AGENT_CONTEXT_LINE.test(rawBody); - if (misplaced) warnings.push({ subject, reason: "misplaced-trailer" }); - records.push({ subject, context: null, malformed: false, ambiguous: false, misplaced }); + if (misplaced) warnings.push({ ...base, reason: "misplaced-trailer" }); + records.push({ ...base, context: null, malformed: false, ambiguous: false, misplaced }); continue; } if (trailerLines.length > 1) { - warnings.push({ subject, reason: "ambiguous-trailer" }); - records.push({ subject, context: null, malformed: false, ambiguous: true, misplaced: false }); + warnings.push({ ...base, reason: "ambiguous-trailer" }); + records.push({ ...base, context: null, malformed: false, ambiguous: true, misplaced: false }); continue; } try { const context: unknown = JSON.parse(trailerLines[0]); - records.push({ subject, context, malformed: false, ambiguous: false, misplaced: false }); + records.push({ ...base, context, malformed: false, ambiguous: false, misplaced: false }); } catch { - warnings.push({ subject, reason: "malformed-json" }); - records.push({ subject, context: null, malformed: true, ambiguous: false, misplaced: false }); + warnings.push({ ...base, reason: "malformed-json" }); + records.push({ ...base, context: null, malformed: true, ambiguous: false, misplaced: false }); } } diff --git a/cli/tests/parser.test.ts b/cli/tests/parser.test.ts index fa6ab1d..0005eea 100644 --- a/cli/tests/parser.test.ts +++ b/cli/tests/parser.test.ts @@ -1,3 +1,4 @@ +import { execFileSync } from "node:child_process"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -7,17 +8,24 @@ import { FIELD_SEP, GitCommandError, LOG_FORMAT, + LOG_FORMAT_FIELD_COUNT, + LogFormatError, NotAGitRepositoryError, parseLog, runGitLog, - SENTINEL, } from "../src/parser.js"; import { isAgentContextObject } from "../src/types.js"; import { TempRepo } from "./fixtures/temp-repo.js"; -/** Builds a synthetic raw log record matching LOG_FORMAT's two NUL-delimited fields, for one commit. Subject is always the raw body's first line, matching how parseLog derives it, so tests don't pass it separately. */ +/** + * Builds one synthetic commit's raw log output, shaped exactly like + * LOG_FORMAT's real output: two NUL-terminated fields (trailer + * extraction, body) followed by git's record-terminating newline. + * Subject is always the raw body's first line, matching how parseLog + * derives it, so tests don't pass it separately. + */ function chunk(trailerSection: string, rawBody: string): string { - return trailerSection + FIELD_SEP + rawBody + SENTINEL; + return [trailerSection, rawBody].map((f) => f + FIELD_SEP).join("") + "\n"; } describe("parseLog", () => { @@ -174,6 +182,11 @@ describe("parseLog", () => { expect(warnings).toHaveLength(2); }); + it("throws LogFormatError on output that isn't a whole number of records, rather than guessing", () => { + expect(() => parseLog("only one field" + FIELD_SEP)).toThrow(LogFormatError); + expect(() => parseLog("not log output at all")).toThrow(LogFormatError); + }); + it("handles an empty log (no commits) without error", () => { const { records, warnings } = parseLog(""); expect(records).toHaveLength(0); @@ -227,6 +240,23 @@ describe("runGitLog + parseLog, against a real temp git repo", () => { expect(warnings).toHaveLength(0); }); + it("keeps an empty-message commit and its neighbors as separate, correct records", () => { + // Regression test for a real bug, reproduced against real git before + // the fix: records used to be split on a "\0\0" boundary, and an + // empty message (--allow-empty-message) produces two adjacent empty + // fields that are byte-identical to that boundary. The commit next to + // it was swallowed and the remains misreported as MISPLACED. + repo = new TempRepo(); + repo.commit('fix: before\n\nAgent-Context: {"why":"kept"}'); + execFileSync("git", ["commit", "-q", "--allow-empty", "--allow-empty-message", "-m", ""], { cwd: repo.dir }); + repo.commit("chore: after"); + const { records, warnings } = parseLog(runGitLog([], repo.dir)); + expect(records.map((r) => r.subject)).toEqual(["chore: after", "", "fix: before"]); + expect(records[1]).toMatchObject({ context: null, misplaced: false }); + expect(records[2].context).toEqual({ why: "kept" }); + expect(warnings).toHaveLength(0); + }); + it("correctly isolates a duplicate-trailer commit from real git output, not just synthetic input", () => { repo = new TempRepo(); repo.commit( @@ -372,13 +402,14 @@ describe("checkGitVersion", () => { // pins the constant's own shape so a future edit can't silently drop a // field without a test noticing. describe("LOG_FORMAT", () => { - it("has exactly two NUL-delimited fields ending in the double-NUL sentinel", () => { + it("emits trailer and body, each NUL-terminated", () => { expect(FIELD_SEP).toBe("\0"); - expect(SENTINEL).toBe("\0\0"); + expect(LOG_FORMAT_FIELD_COUNT).toBe(2); // The format string itself uses git's %x00 escape syntax, not a literal // NUL, since it's passed as a process argument (see LOG_FORMAT's own // comment for why an embedded NUL there is invalid). - expect(LOG_FORMAT).toBe("%(trailers:key=Agent-Context,valueonly)%x00%B%x00%x00"); + expect(LOG_FORMAT).toBe("%(trailers:key=Agent-Context,valueonly)%x00%B%x00"); + expect(LOG_FORMAT.split("%x00")).toHaveLength(LOG_FORMAT_FIELD_COUNT + 1); expect(LOG_FORMAT).not.toContain("\0"); }); }); From 3b95d2356f5745cccedfdfbadd299172a57608d5 Mon Sep 17 00:00:00 2001 From: angelcodes95 Date: Wed, 4 Nov 2026 09:03:07 -0800 Subject: [PATCH 3/5] feat(cli): include commit hash in log, show, and check output Records, check results, and parse warnings now carry hash and shortHash. Text output leads each entry with the short hash, git log --oneline style; --json carries both. Agent-Context: {"why":"Without a hash, a log --grep match could be read but not acted on: there was no way to pass it to git show or agent-commit show, and subjects are not unique. shortHash comes from git's %h rather than slicing %H so it honors core.abbrev and stays unambiguous in large repos. Adding JSON fields is additive, so existing --json consumers keep working.","touches":["cli/src/parser.ts","cli/src/types.ts","cli/src/format.ts","cli/src/check.ts","cli/src/index.ts"]} --- CHANGELOG.md | 8 ++++++++ README.md | 2 +- SPEC.md | 2 +- cli/README.md | 4 ++++ cli/src/check.ts | 7 +++++-- cli/src/format.ts | 8 ++++++-- cli/src/index.ts | 6 ++++-- cli/src/parser.ts | 31 ++++++++++++++++++------------- cli/src/types.ts | 6 ++++++ cli/tests/check.test.ts | 8 +++++--- cli/tests/cli.test.ts | 7 +++++-- cli/tests/fixtures/record.ts | 6 ++++++ cli/tests/format.test.ts | 16 ++++++++-------- cli/tests/parser.test.ts | 34 ++++++++++++++++++++++++---------- 14 files changed, 101 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac1dbfd..9a41dda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ version). ## [Unreleased] +### Added + +- `log`, `show`, and `check` results now include each commit's hash: + `hash` and `shortHash` fields in `--json` output, and the short hash + leading each entry in text output. Previously a result could be found + but not acted on, since nothing identified which commit it was. Parse + warnings on stderr name the commit's short hash too. + ### Fixed - A commit with an empty message (`git commit --allow-empty-message`) diff --git a/README.md b/README.md index 771e14c..c6ee424 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ Prose is exactly what agents already do for free, and it's genuinely fine for a ## What the CLI does today -Reading a single repo's history back already works today with plain git, no tool required (see `SPEC.md`, "Reading it back"). The CLI in this repo (`cli/`, published to npm) adds structured filtering on top: `agent-commit log --breaking`, `--grep` (case-insensitive substring match against `why`), `--refs` (exact match against an entry in `refs`, so it's only as reliable as the canonical form guidance in `SPEC.md`; a repo that mixes `INFRA-441` and `#441` for the same ticket will find neither with a query for the other, silently, no error), `--since`/`--until`, `agent-commit show ` for a single commit, both with a pretty-printed default view and a `--json` mode for machine consumption. No `--type` filter, on purpose, `type` isn't a field this format uses (see SPEC.md). `agent-commit check` reports, read-only, whether recent commits on the current branch have a usable `Agent-Context` trailer, distinguishing a trailer that was never written from one that exists in the message but wasn't recognized as a trailer (`MISPLACED`, see SPEC.md's squash-merge section for why that happens), useful right after a squash-merge to confirm the merge commit actually got one. `agent-commit merge-draft ` deterministically unions a branch's `touches`/`refs`/`breaking` and lists per-commit `why` candidates for a squash-merge trailer. `agent-commit init` writes the `AGENTS.md` block (see "How to use it" below) instead of an agent hand-transcribing it. `agent-commit --version` prints the installed version. Try it now with no install: `npx agent-commit log`. Building from source instead: `cd cli && npm install && npm run build && node dist/index.js log`. +Reading a single repo's history back already works today with plain git, no tool required (see `SPEC.md`, "Reading it back"). The CLI in this repo (`cli/`, published to npm) adds structured filtering on top: `agent-commit log --breaking`, `--grep` (case-insensitive substring match against `why`), `--refs` (exact match against an entry in `refs`, so it's only as reliable as the canonical form guidance in `SPEC.md`; a repo that mixes `INFRA-441` and `#441` for the same ticket will find neither with a query for the other, silently, no error), `--since`/`--until`, `agent-commit show ` for a single commit, both with a pretty-printed default view and a `--json` mode for machine consumption. Every result carries its commit hash (`hash` and `shortHash` in `--json`, the short hash leading each entry in pretty output), so anything found can be handed straight to `git show`. No `--type` filter, on purpose, `type` isn't a field this format uses (see SPEC.md). `agent-commit check` reports, read-only, whether recent commits on the current branch have a usable `Agent-Context` trailer, distinguishing a trailer that was never written from one that exists in the message but wasn't recognized as a trailer (`MISPLACED`, see SPEC.md's squash-merge section for why that happens), useful right after a squash-merge to confirm the merge commit actually got one. `agent-commit merge-draft ` deterministically unions a branch's `touches`/`refs`/`breaking` and lists per-commit `why` candidates for a squash-merge trailer. `agent-commit init` writes the `AGENTS.md` block (see "How to use it" below) instead of an agent hand-transcribing it. `agent-commit --version` prints the installed version. Try it now with no install: `npx agent-commit log`. Building from source instead: `cd cli && npm install && npm run build && node dist/index.js log`. ## What's not built diff --git a/SPEC.md b/SPEC.md index 54741ba..6e7e686 100644 --- a/SPEC.md +++ b/SPEC.md @@ -128,4 +128,4 @@ Split the output on the NUL byte (`\0` in most languages, `tr '\0' '\n'` at a sh This already works today, verified against a real commit, not just described; see `README.md`, "What the CLI does today," for the working tool built on top of it. -The CLI's own internal format goes one step further than the two commands above: it also captures each commit's raw message body, specifically so it can tell a genuinely absent trailer apart from one that exists in the message but landed outside the position git recognizes as a trailer (see "The real trap: placement, not the merge strategy itself" above). That's a CLI capability, not a requirement of the format itself; the plain two-field commands here remain everything a reader needs with nothing installed. +The CLI's own internal format goes one step further than the two commands above: it also captures each commit's hash, so every result can be acted on, and its raw message body, specifically so it can tell a genuinely absent trailer apart from one that exists in the message but landed outside the position git recognizes as a trailer (see "The real trap: placement, not the merge strategy itself" above). That's a CLI capability, not a requirement of the format itself; the plain two-field commands here remain everything a reader needs with nothing installed. diff --git a/cli/README.md b/cli/README.md index 2c4e592..a045361 100644 --- a/cli/README.md +++ b/cli/README.md @@ -47,6 +47,10 @@ form consistency matters. `agent-commit show [--json]` Shows a single commit's `Agent-Context` trailer. +Every result from `log`, `show`, and `check` includes the commit's hash: +`hash` (full) and `shortHash` (git's own abbreviation) in `--json` +output, and the short hash at the start of each entry in text output. + `agent-commit check [--range ] [--count ] [--json]` Read-only, advisory report on whether recent commits have a usable trailer. Distinguishes a commit with no trailer at all from one where an diff --git a/cli/src/check.ts b/cli/src/check.ts index 972ac68..9576359 100644 --- a/cli/src/check.ts +++ b/cli/src/check.ts @@ -4,6 +4,8 @@ import type { AgentCommitRecord } from "./types.js"; export type CheckStatus = "ok" | "miss" | "misplaced"; export interface CheckResult { + hash: string; + shortHash: string; subject: string; status: CheckStatus; } @@ -20,8 +22,9 @@ export interface CheckResult { */ export function checkRecords(records: AgentCommitRecord[]): CheckResult[] { return records.map((record) => { + const base = { hash: record.hash, shortHash: record.shortHash, subject: record.subject }; if (record.misplaced) { - return { subject: record.subject, status: "misplaced" }; + return { ...base, status: "misplaced" }; } // "ok" requires an object-shaped trailer, the same bar query.ts's // structured filters use (isAgentContextObject). SPEC.md allows any @@ -32,6 +35,6 @@ export function checkRecords(records: AgentCommitRecord[]): CheckResult[] { // a false green light query.ts could never act on. Malformed and // ambiguous trailers already fall through to miss below. const ok = isAgentContextObject(record.context) && !record.malformed && !record.ambiguous; - return { subject: record.subject, status: ok ? "ok" : "miss" }; + return { ...base, status: ok ? "ok" : "miss" }; }); } diff --git a/cli/src/format.ts b/cli/src/format.ts index 611e179..0a8d071 100644 --- a/cli/src/format.ts +++ b/cli/src/format.ts @@ -115,12 +115,16 @@ function formatContextFields(context: Record): string[] { } /** - * Renders one record as human-readable text. Field order favors SPEC.md's + * Renders one record as human-readable text: short hash and subject, then + * the trailer's fields. Field order favors SPEC.md's * common fields but never drops an unrecognized key, the format has no * required schema, so the renderer can't assume one either. */ export function formatPretty(record: AgentCommitRecord): string { - const lines = [sanitizeForTerminal(record.subject)]; + // Hash first, `git log --oneline` style, so every result can be handed + // straight to `git show` or `agent-commit show`. Hashes are git's own + // hex output, never commit content, so they need no sanitizing. + const lines = [`${record.shortHash} ${sanitizeForTerminal(record.subject)}`]; if (record.malformed) { lines.push(" (malformed Agent-Context trailer, could not parse as JSON)"); diff --git a/cli/src/index.ts b/cli/src/index.ts index 6100330..6b16c36 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -59,7 +59,7 @@ function runFatal(fn: () => T): T { function printWarnings(warnings: ParseWarning[]): void { for (const warning of warnings) { process.stderr.write( - `agent-commit: warning: ${warning.reason} on commit "${sanitizeForTerminal(warning.subject)}", skipped\n`, + `agent-commit: warning: ${warning.reason} on commit ${warning.shortHash} "${sanitizeForTerminal(warning.subject)}", skipped\n`, ); } } @@ -186,7 +186,9 @@ program misplaced: "MISPLACED".padEnd(labelWidth), }; for (const result of results) { - process.stdout.write(`${labels[result.status]} ${sanitizeForTerminal(result.subject)}\n`); + process.stdout.write( + `${labels[result.status]} ${result.shortHash} ${sanitizeForTerminal(result.subject)}\n`, + ); } } diff --git a/cli/src/parser.ts b/cli/src/parser.ts index 7eec3b4..0a7fa1f 100644 --- a/cli/src/parser.ts +++ b/cli/src/parser.ts @@ -12,8 +12,8 @@ import type { AgentCommitRecord, ParseResult, ParseWarning } from "./types.js"; */ export const FIELD_SEP = "\0"; /** - * Fields emitted per commit by LOG_FORMAT, in order: trailer extraction, - * raw body. Records are parsed + * Fields emitted per commit by LOG_FORMAT, in order: full hash, + * abbreviated hash, trailer extraction, raw body. Records are parsed * positionally, LOG_FORMAT_FIELD_COUNT fields at a time, rather than split * on a multi-byte record boundary: any field can legitimately be empty (no * trailer, or a commit made with --allow-empty-message), and two adjacent @@ -21,16 +21,19 @@ export const FIELD_SEP = "\0"; * is exactly how an empty-message commit used to swallow the record next * to it. */ -export const LOG_FORMAT_FIELD_COUNT = 2; +export const LOG_FORMAT_FIELD_COUNT = 4; /** * Internal to the CLI, deliberately richer than the plain-git example in * SPEC.md's "Reading it back" section. That example stays a minimal * two-field command anyone can run with no tool; this format adds the - * raw commit body (so the CLI can detect a trailer that's present in the - * message but sitting outside git's recognized trailer block, see the - * MISPLACED detection in parseLog below). The subject is derived from the - * body's first line rather than fetched separately, one fewer field that - * could disagree with itself. + * commit hash (so every result can be acted on: `git show`, `blame`, + * `agent-commit show`) and the raw commit body (so the CLI can detect a + * trailer that's present in the message but sitting outside git's + * recognized trailer block, see the MISPLACED detection in parseLog + * below). The subject is derived from the body's first line rather than + * fetched separately, one fewer field that could disagree with itself. + * The abbreviated hash comes from git (%h) rather than slicing %H, so it + * honors core.abbrev and stays unique in large repos. * * Uses `%x00`, git's own escape syntax for emitting a NUL byte, not a * literal NUL character in this string: this format string is passed as a @@ -40,7 +43,7 @@ export const LOG_FORMAT_FIELD_COUNT = 2; * with no such restriction, that output byte is what FIELD_SEP above * matches against in parseLog. */ -export const LOG_FORMAT = "%(trailers:key=Agent-Context,valueonly)%x00%B%x00"; +export const LOG_FORMAT = "%H%x00%h%x00%(trailers:key=Agent-Context,valueonly)%x00%B%x00"; // Tolerates optional whitespace before the colon (git itself does: `Agent-Context : {...}` // is extracted as a trailer just as readily as `Agent-Context: {...}`, verified against real // git) and is case-insensitive to match git's own case-insensitive trailer key matching. @@ -192,12 +195,14 @@ export function parseLog(raw: string): ParseResult { for (let i = 0; i < fields.length; i += LOG_FORMAT_FIELD_COUNT) { // git separates records with a newline, which lands at the start of - // the next record's first field; trailer values never start with one. - const trailerSection = fields[i].replace(/^\n/, ""); - const rawBody = fields[i + 1].replace(/\n+$/, ""); + // the next record's first field; hashes never contain whitespace. + const hash = fields[i].trim(); + const shortHash = fields[i + 1]; + const trailerSection = fields[i + 2]; + const rawBody = fields[i + 3].replace(/\n+$/, ""); const subject = rawBody.split("\n")[0] ?? ""; const trailerLines = trailerSection.split("\n").filter((line) => line.trim() !== ""); - const base = { subject }; + const base = { hash, shortHash, subject }; if (trailerLines.length === 0) { const misplaced = AGENT_CONTEXT_LINE.test(rawBody); diff --git a/cli/src/types.ts b/cli/src/types.ts index bccea7f..46a4435 100644 --- a/cli/src/types.ts +++ b/cli/src/types.ts @@ -2,6 +2,10 @@ export type AgentContextValue = Record; export interface AgentCommitRecord { + /** The commit's full object hash. */ + hash: string; + /** git's own abbreviated hash (%h), unique within the repo at the time of reading. For display; use `hash` for anything machine-facing. */ + shortHash: string; /** The commit's subject line. */ subject: string; /** @@ -33,6 +37,8 @@ export function isAgentContextObject(value: unknown): value is AgentContextValue } export interface ParseWarning { + hash: string; + shortHash: string; subject: string; reason: "malformed-json" | "ambiguous-trailer" | "misplaced-trailer"; } diff --git a/cli/tests/check.test.ts b/cli/tests/check.test.ts index e93c43f..aeb13a8 100644 --- a/cli/tests/check.test.ts +++ b/cli/tests/check.test.ts @@ -1,11 +1,13 @@ import { describe, expect, it } from "vitest"; import { checkRecords } from "../src/check.js"; -import { record } from "./fixtures/record.js"; +import { FAKE_HASH, FAKE_SHORT_HASH, record } from "./fixtures/record.js"; + +const fakeHashes = { hash: FAKE_HASH, shortHash: FAKE_SHORT_HASH }; describe("checkRecords", () => { it("marks a commit with a valid object trailer as ok", () => { const [result] = checkRecords([record({ subject: "fix: x", context: { why: "y" } })]); - expect(result).toEqual({ subject: "fix: x", status: "ok" }); + expect(result).toEqual({ ...fakeHashes, subject: "fix: x", status: "ok" }); }); it("marks a commit with valid-but-non-object JSON as miss, ok requires an object shape", () => { @@ -15,7 +17,7 @@ describe("checkRecords", () => { it("marks a commit with no trailer at all as miss", () => { const [result] = checkRecords([record({ subject: "chore: bump", context: null })]); - expect(result).toEqual({ subject: "chore: bump", status: "miss" }); + expect(result).toEqual({ ...fakeHashes, subject: "chore: bump", status: "miss" }); }); it("marks a malformed trailer as miss, it's not usable even though something was written", () => { diff --git a/cli/tests/cli.test.ts b/cli/tests/cli.test.ts index b6c7249..fd3214e 100644 --- a/cli/tests/cli.test.ts +++ b/cli/tests/cli.test.ts @@ -35,7 +35,7 @@ describe("agent-commit log, against the compiled binary", () => { repo.commit('fix: real one\n\nAgent-Context: {"why":"a real reason"}'); const { stdout, status } = runCli(["log"], repo.dir); expect(status).toBe(0); - expect(stdout).toContain("fix: real one"); + expect(stdout).toMatch(/^[0-9a-f]{7,} fix: real one$/m); expect(stdout).toContain("why: a real reason"); }); @@ -46,6 +46,7 @@ describe("agent-commit log, against the compiled binary", () => { expect(status).toBe(0); const parsed = JSON.parse(stdout); expect(parsed[0].subject).toBe("fix: json one"); + expect(parsed[0].hash).toMatch(/^[0-9a-f]{40}$/); expect(parsed[0].context).toEqual({ why: "x" }); }); @@ -166,7 +167,9 @@ describe("agent-commit check, against the compiled binary", () => { const { stdout, status } = runCli(["check", "--json"], repo.dir); expect(status).toBe(0); const parsed = JSON.parse(stdout); - expect(parsed).toEqual([{ subject: "fix: fine", status: "ok" }]); + const hash = execFileSync("git", ["rev-parse", "HEAD"], { cwd: repo.dir, encoding: "utf8" }).trim(); + const shortHash = execFileSync("git", ["rev-parse", "--short", "HEAD"], { cwd: repo.dir, encoding: "utf8" }).trim(); + expect(parsed).toEqual([{ hash, shortHash, subject: "fix: fine", status: "ok" }]); }); it("prints the MISPLACED explanation to stderr only when something is actually misplaced", () => { diff --git a/cli/tests/fixtures/record.ts b/cli/tests/fixtures/record.ts index c872f2b..df7462d 100644 --- a/cli/tests/fixtures/record.ts +++ b/cli/tests/fixtures/record.ts @@ -1,8 +1,14 @@ import type { AgentCommitRecord } from "../../src/types.js"; +/** A fixed, obviously fake hash for synthetic records. Tests that care about real hashes use TempRepo instead. */ +export const FAKE_HASH = "0123456789abcdef0123456789abcdef01234567"; +export const FAKE_SHORT_HASH = "0123456"; + /** Builds a synthetic AgentCommitRecord with neutral defaults, overriding only what a test cares about. */ export function record(overrides: Partial = {}): AgentCommitRecord { return { + hash: FAKE_HASH, + shortHash: FAKE_SHORT_HASH, subject: "subject", context: null, malformed: false, diff --git a/cli/tests/format.test.ts b/cli/tests/format.test.ts index d915ba8..0c98c42 100644 --- a/cli/tests/format.test.ts +++ b/cli/tests/format.test.ts @@ -1,11 +1,11 @@ import { describe, expect, it } from "vitest"; import { formatJson, formatPretty, formatPrettyList, sanitizeForTerminal } from "../src/format.js"; -import { record } from "./fixtures/record.js"; +import { FAKE_SHORT_HASH, record } from "./fixtures/record.js"; describe("formatPretty", () => { - it("prints just the subject when there's no trailer", () => { + it("prints just the short hash and subject when there's no trailer", () => { const out = formatPretty(record({ subject: "chore: bump dependency" })); - expect(out).toBe("chore: bump dependency"); + expect(out).toBe(`${FAKE_SHORT_HASH} chore: bump dependency`); }); it("prints common fields in the documented order, not object insertion order", () => { @@ -18,7 +18,7 @@ describe("formatPretty", () => { }), ); const lines = out.split("\n"); - expect(lines[0]).toBe("fix: retry backoff"); + expect(lines[0]).toBe(`${FAKE_SHORT_HASH} fix: retry backoff`); expect(lines[1]).toBe(" why: added exponential backoff"); expect(lines[2]).toBe(" breaking: false"); }); @@ -67,7 +67,7 @@ describe("formatPretty", () => { it("renders valid-but-non-object JSON (a bare string) without crashing or mislabeling it", () => { const out = formatPretty(record({ subject: "fix: odd", context: "just a string" })); - expect(out).toBe("fix: odd\n just a string"); + expect(out).toBe(`${FAKE_SHORT_HASH} fix: odd\n just a string`); }); it("neutralizes a right-to-left override in a why value instead of applying it to the terminal", () => { @@ -118,10 +118,10 @@ describe("formatPretty", () => { describe("formatPrettyList", () => { it("joins multiple records with a blank line between them", () => { const out = formatPrettyList([ - record({ subject: "first" }), - record({ subject: "second", context: { why: "reason" } }), + record({ shortHash: "aaaaaaa", subject: "first" }), + record({ shortHash: "bbbbbbb", subject: "second", context: { why: "reason" } }), ]); - expect(out).toBe("first\n\nsecond\n why: reason"); + expect(out).toBe("aaaaaaa first\n\nbbbbbbb second\n why: reason"); }); it("returns an empty string for an empty list, not a stray separator", () => { diff --git a/cli/tests/parser.test.ts b/cli/tests/parser.test.ts index 0005eea..e08504b 100644 --- a/cli/tests/parser.test.ts +++ b/cli/tests/parser.test.ts @@ -15,25 +15,29 @@ import { runGitLog, } from "../src/parser.js"; import { isAgentContextObject } from "../src/types.js"; +import { FAKE_HASH, FAKE_SHORT_HASH } from "./fixtures/record.js"; import { TempRepo } from "./fixtures/temp-repo.js"; /** * Builds one synthetic commit's raw log output, shaped exactly like - * LOG_FORMAT's real output: two NUL-terminated fields (trailer - * extraction, body) followed by git's record-terminating newline. + * LOG_FORMAT's real output: four NUL-terminated fields (hash, short hash, + * trailer extraction, body) followed by git's record-terminating newline. * Subject is always the raw body's first line, matching how parseLog * derives it, so tests don't pass it separately. */ function chunk(trailerSection: string, rawBody: string): string { - return [trailerSection, rawBody].map((f) => f + FIELD_SEP).join("") + "\n"; + return [FAKE_HASH, FAKE_SHORT_HASH, trailerSection, rawBody].map((f) => f + FIELD_SEP).join("") + "\n"; } +const fakeHashes = { hash: FAKE_HASH, shortHash: FAKE_SHORT_HASH }; + describe("parseLog", () => { it("returns context: null for a commit with no trailer", () => { const raw = chunk("", "chore: no trailer"); const { records, warnings } = parseLog(raw); expect(records).toHaveLength(1); expect(records[0]).toEqual({ + ...fakeHashes, subject: "chore: no trailer", context: null, malformed: false, @@ -67,7 +71,7 @@ describe("parseLog", () => { expect(records[0].context).toBeNull(); expect(records[0].malformed).toBe(true); expect(records[0].misplaced).toBe(false); - expect(warnings).toEqual([{ subject: "fix: broken trailer", reason: "malformed-json" }]); + expect(warnings).toEqual([{ ...fakeHashes, subject: "fix: broken trailer", reason: "malformed-json" }]); }); it("flags duplicate Agent-Context trailers as ambiguous rather than picking one", () => { @@ -80,7 +84,7 @@ describe("parseLog", () => { expect(records[0].context).toBeNull(); expect(records[0].ambiguous).toBe(true); expect(records[0].misplaced).toBe(false); - expect(warnings).toEqual([{ subject: "fix: two trailers", reason: "ambiguous-trailer" }]); + expect(warnings).toEqual([{ ...fakeHashes, subject: "fix: two trailers", reason: "ambiguous-trailer" }]); }); it("flags a trailer that's present in the body but not recognized as a trailer as misplaced", () => { @@ -98,7 +102,7 @@ describe("parseLog", () => { expect(records[0].malformed).toBe(false); expect(records[0].ambiguous).toBe(false); expect(records[0].misplaced).toBe(true); - expect(warnings).toEqual([{ subject: "fix: github prefill shape", reason: "misplaced-trailer" }]); + expect(warnings).toEqual([{ ...fakeHashes, subject: "fix: github prefill shape", reason: "misplaced-trailer" }]); }); it("does not flag misplaced when there's genuinely no Agent-Context text anywhere in the body", () => { @@ -183,7 +187,7 @@ describe("parseLog", () => { }); it("throws LogFormatError on output that isn't a whole number of records, rather than guessing", () => { - expect(() => parseLog("only one field" + FIELD_SEP)).toThrow(LogFormatError); + expect(() => parseLog(FAKE_HASH + FIELD_SEP + FAKE_SHORT_HASH + FIELD_SEP)).toThrow(LogFormatError); expect(() => parseLog("not log output at all")).toThrow(LogFormatError); }); @@ -240,6 +244,16 @@ describe("runGitLog + parseLog, against a real temp git repo", () => { expect(warnings).toHaveLength(0); }); + it("reports each commit's real full and abbreviated hash", () => { + repo = new TempRepo(); + repo.commit('fix: hashed\n\nAgent-Context: {"why":"x"}'); + const expectedHash = execFileSync("git", ["rev-parse", "HEAD"], { cwd: repo.dir, encoding: "utf8" }).trim(); + const expectedShort = execFileSync("git", ["rev-parse", "--short", "HEAD"], { cwd: repo.dir, encoding: "utf8" }).trim(); + const { records } = parseLog(runGitLog([], repo.dir)); + expect(records[0].hash).toBe(expectedHash); + expect(records[0].shortHash).toBe(expectedShort); + }); + it("keeps an empty-message commit and its neighbors as separate, correct records", () => { // Regression test for a real bug, reproduced against real git before // the fix: records used to be split on a "\0\0" boundary, and an @@ -402,13 +416,13 @@ describe("checkGitVersion", () => { // pins the constant's own shape so a future edit can't silently drop a // field without a test noticing. describe("LOG_FORMAT", () => { - it("emits trailer and body, each NUL-terminated", () => { + it("emits hash, short hash, trailer, and body, each NUL-terminated", () => { expect(FIELD_SEP).toBe("\0"); - expect(LOG_FORMAT_FIELD_COUNT).toBe(2); + expect(LOG_FORMAT_FIELD_COUNT).toBe(4); // The format string itself uses git's %x00 escape syntax, not a literal // NUL, since it's passed as a process argument (see LOG_FORMAT's own // comment for why an embedded NUL there is invalid). - expect(LOG_FORMAT).toBe("%(trailers:key=Agent-Context,valueonly)%x00%B%x00"); + expect(LOG_FORMAT).toBe("%H%x00%h%x00%(trailers:key=Agent-Context,valueonly)%x00%B%x00"); expect(LOG_FORMAT.split("%x00")).toHaveLength(LOG_FORMAT_FIELD_COUNT + 1); expect(LOG_FORMAT).not.toContain("\0"); }); From ded1c889a2b22fc7a597fb598dd4ebdb752f6bad Mon Sep 17 00:00:00 2001 From: angelcodes95 Date: Wed, 4 Nov 2026 09:04:15 -0800 Subject: [PATCH 4/5] feat(cli): add --touches filter to log Matches the touches field on whole path segments, in both directions: an entry inside the queried path or one containing it both match, a partial segment never does. Agent-Context: {"why":"Exact match, like --refs, would miss the common case of asking about a file when the commit listed its directory. Plain string-prefix match was rejected because src/auth would then match src/authz. Matching only entries under the query was rejected too: a commit claiming it touched src is relevant to a question about src/auth/session.ts.","touches":["cli/src/query.ts","cli/src/index.ts"]} --- CHANGELOG.md | 4 +++ README.md | 2 +- cli/README.md | 6 +++-- cli/src/index.ts | 6 +++++ cli/src/query.ts | 50 +++++++++++++++++++++++++++++++++++++- cli/tests/cli.test.ts | 9 +++++++ cli/tests/query.test.ts | 54 ++++++++++++++++++++++++++++++++++++++++- 7 files changed, 126 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a41dda..41c61f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ version). leading each entry in text output. Previously a result could be found but not acted on, since nothing identified which commit it was. Parse warnings on stderr name the commit's short hash too. +- `log --touches `: filters by the `touches` field, matching whole + path segments in both directions, so `src/auth` finds commits listing + `src/auth`, `src/auth/session.ts`, or the broader `src`, but never + `src/authz`. Leading `./` or `/` and trailing `/` are ignored. ### Fixed diff --git a/README.md b/README.md index c6ee424..bf29c76 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ Prose is exactly what agents already do for free, and it's genuinely fine for a ## What the CLI does today -Reading a single repo's history back already works today with plain git, no tool required (see `SPEC.md`, "Reading it back"). The CLI in this repo (`cli/`, published to npm) adds structured filtering on top: `agent-commit log --breaking`, `--grep` (case-insensitive substring match against `why`), `--refs` (exact match against an entry in `refs`, so it's only as reliable as the canonical form guidance in `SPEC.md`; a repo that mixes `INFRA-441` and `#441` for the same ticket will find neither with a query for the other, silently, no error), `--since`/`--until`, `agent-commit show ` for a single commit, both with a pretty-printed default view and a `--json` mode for machine consumption. Every result carries its commit hash (`hash` and `shortHash` in `--json`, the short hash leading each entry in pretty output), so anything found can be handed straight to `git show`. No `--type` filter, on purpose, `type` isn't a field this format uses (see SPEC.md). `agent-commit check` reports, read-only, whether recent commits on the current branch have a usable `Agent-Context` trailer, distinguishing a trailer that was never written from one that exists in the message but wasn't recognized as a trailer (`MISPLACED`, see SPEC.md's squash-merge section for why that happens), useful right after a squash-merge to confirm the merge commit actually got one. `agent-commit merge-draft ` deterministically unions a branch's `touches`/`refs`/`breaking` and lists per-commit `why` candidates for a squash-merge trailer. `agent-commit init` writes the `AGENTS.md` block (see "How to use it" below) instead of an agent hand-transcribing it. `agent-commit --version` prints the installed version. Try it now with no install: `npx agent-commit log`. Building from source instead: `cd cli && npm install && npm run build && node dist/index.js log`. +Reading a single repo's history back already works today with plain git, no tool required (see `SPEC.md`, "Reading it back"). The CLI in this repo (`cli/`, published to npm) adds structured filtering on top: `agent-commit log --breaking`, `--grep` (case-insensitive substring match against `why`), `--refs` (exact match against an entry in `refs`, so it's only as reliable as the canonical form guidance in `SPEC.md`; a repo that mixes `INFRA-441` and `#441` for the same ticket will find neither with a query for the other, silently, no error), `--touches` (path match against `touches` on whole segments, in both directions: `--touches src/auth` finds commits listing `src/auth`, `src/auth/session.ts`, or the broader `src`, never `src/authz`), `--since`/`--until`, `agent-commit show ` for a single commit, both with a pretty-printed default view and a `--json` mode for machine consumption. Every result carries its commit hash (`hash` and `shortHash` in `--json`, the short hash leading each entry in pretty output), so anything found can be handed straight to `git show`. No `--type` filter, on purpose, `type` isn't a field this format uses (see SPEC.md). `agent-commit check` reports, read-only, whether recent commits on the current branch have a usable `Agent-Context` trailer, distinguishing a trailer that was never written from one that exists in the message but wasn't recognized as a trailer (`MISPLACED`, see SPEC.md's squash-merge section for why that happens), useful right after a squash-merge to confirm the merge commit actually got one. `agent-commit merge-draft ` deterministically unions a branch's `touches`/`refs`/`breaking` and lists per-commit `why` candidates for a squash-merge trailer. `agent-commit init` writes the `AGENTS.md` block (see "How to use it" below) instead of an agent hand-transcribing it. `agent-commit --version` prints the installed version. Try it now with no install: `npx agent-commit log`. Building from source instead: `cd cli && npm install && npm run build && node dist/index.js log`. ## What's not built diff --git a/cli/README.md b/cli/README.md index a045361..5dc6d54 100644 --- a/cli/README.md +++ b/cli/README.md @@ -38,11 +38,13 @@ target directory (the current directory if none is given), and links an existing, unrelated commit-message convention or an already-set-up repo without `--force`/`--update`. -`agent-commit log [--breaking] [--grep ] [--refs ] [--since ] [--until ] [--json]` +`agent-commit log [--breaking] [--grep ] [--refs ] [--touches ] [--since ] [--until ] [--json]` Filters commit history by `Agent-Context` fields. `--grep` is a case-insensitive substring match against `why`, not semantic search. `--refs` is an exact match, see the main repo's `SPEC.md` for why ref -form consistency matters. +form consistency matters. `--touches` matches whole path segments in +both directions: `src/auth` finds `src/auth`, `src/auth/session.ts`, +and the broader `src`, but never `src/authz`. `agent-commit show [--json]` Shows a single commit's `Agent-Context` trailer. diff --git a/cli/src/index.ts b/cli/src/index.ts index 6b16c36..67f32ce 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -91,6 +91,10 @@ program .option("--breaking", "filter to breaking: true only") .option("--grep ", "case-insensitive substring match against why") .option("--refs ", "filter to commits whose refs array contains this exact value") + .option( + "--touches ", + "filter to commits whose touches overlaps this path, on whole path segments (src/auth matches src/auth/session.ts and src)", + ) .option("--since ", "passed through to git log --since") .option("--until ", "passed through to git log --until") .option("--json", "output machine-readable JSON instead of pretty text") @@ -98,6 +102,7 @@ program breaking?: boolean; grep?: string; refs?: string; + touches?: string; since?: string; until?: string; json?: boolean; @@ -116,6 +121,7 @@ program if (opts.breaking) queryOptions.breaking = true; if (opts.grep) queryOptions.grep = opts.grep; if (opts.refs) queryOptions.refs = opts.refs; + if (opts.touches !== undefined) queryOptions.touches = opts.touches; const filtered = queryRecords(records, queryOptions); diff --git a/cli/src/query.ts b/cli/src/query.ts index 31b25f0..f3a6fc6 100644 --- a/cli/src/query.ts +++ b/cli/src/query.ts @@ -8,10 +8,51 @@ export interface QueryOptions { grep?: string; /** Exact match against an entry in context.refs. */ refs?: string; + /** Path match against context.touches, on whole path segments; see touchesPath. */ + touches?: string; } function hasAnyFilter(options: QueryOptions): boolean { - return options.breaking !== undefined || options.grep !== undefined || options.refs !== undefined; + return ( + options.breaking !== undefined || + options.grep !== undefined || + options.refs !== undefined || + options.touches !== undefined + ); +} + +/** + * Reduces a path to the canonical form SPEC.md asks `touches` to use + * (repo-relative, no leading `./` or `/`), plus no trailing `/`, and + * splits it into segments. Applied to both the query and each entry, + * since an entry written slightly off-spec is still a real claim about + * what the commit touched. + */ +function pathSegments(path: string): string[] { + return path + .trim() + .replace(/^(\.\/|\/)+/, "") + .split("/") + .filter((segment) => segment !== ""); +} + +function isSegmentPrefix(prefix: string[], of: string[]): boolean { + return prefix.length <= of.length && prefix.every((segment, i) => segment === of[i]); +} + +/** + * True if a `touches` entry and the queried path overlap, on whole path + * segments, in either direction: an entry inside the queried path + * (`src/auth/session.ts` for `src/auth`) and an entry containing it + * (`src` for `src/auth/session.ts`, a commit claiming it touched the + * broader area) both match. A partial segment never does: `src/au` + * matches nothing under `src/auth`. Case-sensitive, like git paths. + */ +export function touchesPath(entry: string, query: string): boolean { + const entrySegments = pathSegments(entry); + const querySegments = pathSegments(query); + if (entrySegments.length === 0 || querySegments.length === 0) return false; + return isSegmentPrefix(entrySegments, querySegments) || isSegmentPrefix(querySegments, entrySegments); } /** @@ -48,6 +89,13 @@ export function queryRecords( if (!ctx.refs.includes(options.refs)) return false; } + if (options.touches !== undefined) { + const query = options.touches; + if (!Array.isArray(ctx.touches)) return false; + const matches = ctx.touches.some((entry) => typeof entry === "string" && touchesPath(entry, query)); + if (!matches) return false; + } + return true; }); } diff --git a/cli/tests/cli.test.ts b/cli/tests/cli.test.ts index fd3214e..70c1cac 100644 --- a/cli/tests/cli.test.ts +++ b/cli/tests/cli.test.ts @@ -50,6 +50,15 @@ describe("agent-commit log, against the compiled binary", () => { expect(parsed[0].context).toEqual({ why: "x" }); }); + it("filters by --touches on path segments", () => { + repo = new TempRepo(); + repo.commit('feat: auth\n\nAgent-Context: {"why":"a","touches":["src/auth/session.ts"]}'); + repo.commit('feat: authz\n\nAgent-Context: {"why":"b","touches":["src/authz"]}'); + const { stdout, status } = runCli(["log", "--touches", "src/auth", "--json"], repo.dir); + expect(status).toBe(0); + expect(JSON.parse(stdout).map((r: { subject: string }) => r.subject)).toEqual(["feat: auth"]); + }); + it("routes parse warnings to stderr, never mixed into stdout's structured output", () => { repo = new TempRepo(); repo.commit('fix: broken\n\nAgent-Context: {not valid json}'); diff --git a/cli/tests/query.test.ts b/cli/tests/query.test.ts index fff5020..3aea77f 100644 --- a/cli/tests/query.test.ts +++ b/cli/tests/query.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { queryRecords } from "../src/query.js"; +import { queryRecords, touchesPath } from "../src/query.js"; import type { AgentCommitRecord } from "../src/types.js"; import { record } from "./fixtures/record.js"; @@ -97,3 +97,55 @@ describe("queryRecords", () => { expect(subjects).not.toContain("fix: ambiguous trailer"); }); }); + +describe("touches filtering", () => { + const touchFixtures = [ + record({ subject: "a: exact dir", context: { touches: ["src/auth"] } }), + record({ subject: "b: file inside", context: { touches: ["src/auth/session.ts", "docs"] } }), + record({ subject: "c: broader dir", context: { touches: ["src"] } }), + record({ subject: "d: sibling with shared prefix", context: { touches: ["src/authz"] } }), + record({ subject: "e: unrelated", context: { touches: ["cli/src/auth"] } }), + record({ subject: "f: no touches", context: { why: "x" } }), + record({ subject: "g: touches not an array", context: { touches: "src/auth" } }), + ]; + const subjects = (touches: string) => queryRecords(touchFixtures, { touches }).map((r) => r.subject); + + it("matches entries inside, equal to, or containing the queried path", () => { + expect(subjects("src/auth")).toEqual(["a: exact dir", "b: file inside", "c: broader dir"]); + }); + + it("matches a file query against a directory entry that contains it", () => { + expect(subjects("src/auth/session.ts")).toEqual(["a: exact dir", "b: file inside", "c: broader dir"]); + }); + + it("never matches on a partial path segment", () => { + expect(subjects("src/au")).toEqual(["c: broader dir"]); + expect(subjects("src/auth")).not.toContain("d: sibling with shared prefix"); + }); + + it("is anchored at the repo root, not a suffix match", () => { + expect(subjects("src/auth")).not.toContain("e: unrelated"); + }); + + it("tolerates ./, leading /, and trailing / on both the query and the entry", () => { + expect(touchesPath("./src/auth/", "/src/auth")).toBe(true); + expect(subjects("./src/auth/")).toEqual(subjects("src/auth")); + }); + + it("never matches an empty or root-only query, rather than matching everything", () => { + expect(subjects("")).toEqual([]); + expect(subjects("/")).toEqual([]); + expect(touchesPath("", "src")).toBe(false); + }); + + it("is case-sensitive, like git paths", () => { + expect(subjects("SRC/auth")).toEqual([]); + }); + + it("skips records without a touches array, and non-string entries", () => { + expect(subjects("src/auth")).not.toContain("f: no touches"); + expect(subjects("src/auth")).not.toContain("g: touches not an array"); + const mixed = [record({ subject: "h", context: { touches: [42, null, "src/auth"] } })]; + expect(queryRecords(mixed, { touches: "src/auth" })).toHaveLength(1); + }); +}); From bb55fc2ba2a2cf1e15dbedd9a1310747b9755cde Mon Sep 17 00:00:00 2001 From: angelcodes95 Date: Wed, 4 Nov 2026 09:07:43 -0800 Subject: [PATCH 5/5] feat(cli): add blame command for line-level context lookup agent-commit blame [-L [,]] runs git blame and prints the Agent-Context of each commit responsible, with the line ranges it owns. Extracts runGit from runGitLog so both share the same error translation. Agent-Context: {"why":"This is the lookup the README names as the main use case, and until now it took git blame plus a manual git show per hash. Mirrors git's -L syntax instead of file:line because colons are legal in filenames and appear in Windows paths; only the numeric -L forms are accepted so nothing but plain numbers reaches git. Reads the working tree like plain git blame, so uncommitted lines are counted on stderr rather than silently dropped.","touches":["cli/src/blame.ts","cli/src/index.ts","cli/src/parser.ts"]} --- CHANGELOG.md | 5 ++ README.md | 2 +- cli/README.md | 9 +- cli/src/blame.ts | 139 ++++++++++++++++++++++++++++++ cli/src/index.ts | 32 +++++++ cli/src/parser.ts | 12 ++- cli/tests/blame.test.ts | 147 ++++++++++++++++++++++++++++++++ cli/tests/cli.test.ts | 35 ++++++++ cli/tests/fixtures/temp-repo.ts | 12 +++ 9 files changed, 390 insertions(+), 3 deletions(-) create mode 100644 cli/src/blame.ts create mode 100644 cli/tests/blame.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 41c61f5..15ab487 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,11 @@ version). path segments in both directions, so `src/auth` finds commits listing `src/auth`, `src/auth/session.ts`, or the broader `src`, but never `src/authz`. Leading `./` or `/` and trailing `/` are ignored. +- `agent-commit blame [-L [,]]`: runs `git blame` on + the given lines (the whole file without `-L`) and shows the + `Agent-Context` of each commit responsible, with the line ranges each + one owns, in pretty or `--json` form. Lines not committed yet are + counted on stderr instead of being attributed. ### Fixed diff --git a/README.md b/README.md index bf29c76..366710f 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ Prose is exactly what agents already do for free, and it's genuinely fine for a ## What the CLI does today -Reading a single repo's history back already works today with plain git, no tool required (see `SPEC.md`, "Reading it back"). The CLI in this repo (`cli/`, published to npm) adds structured filtering on top: `agent-commit log --breaking`, `--grep` (case-insensitive substring match against `why`), `--refs` (exact match against an entry in `refs`, so it's only as reliable as the canonical form guidance in `SPEC.md`; a repo that mixes `INFRA-441` and `#441` for the same ticket will find neither with a query for the other, silently, no error), `--touches` (path match against `touches` on whole segments, in both directions: `--touches src/auth` finds commits listing `src/auth`, `src/auth/session.ts`, or the broader `src`, never `src/authz`), `--since`/`--until`, `agent-commit show ` for a single commit, both with a pretty-printed default view and a `--json` mode for machine consumption. Every result carries its commit hash (`hash` and `shortHash` in `--json`, the short hash leading each entry in pretty output), so anything found can be handed straight to `git show`. No `--type` filter, on purpose, `type` isn't a field this format uses (see SPEC.md). `agent-commit check` reports, read-only, whether recent commits on the current branch have a usable `Agent-Context` trailer, distinguishing a trailer that was never written from one that exists in the message but wasn't recognized as a trailer (`MISPLACED`, see SPEC.md's squash-merge section for why that happens), useful right after a squash-merge to confirm the merge commit actually got one. `agent-commit merge-draft ` deterministically unions a branch's `touches`/`refs`/`breaking` and lists per-commit `why` candidates for a squash-merge trailer. `agent-commit init` writes the `AGENTS.md` block (see "How to use it" below) instead of an agent hand-transcribing it. `agent-commit --version` prints the installed version. Try it now with no install: `npx agent-commit log`. Building from source instead: `cd cli && npm install && npm run build && node dist/index.js log`. +Reading a single repo's history back already works today with plain git, no tool required (see `SPEC.md`, "Reading it back"). The CLI in this repo (`cli/`, published to npm) adds structured filtering on top: `agent-commit log --breaking`, `--grep` (case-insensitive substring match against `why`), `--refs` (exact match against an entry in `refs`, so it's only as reliable as the canonical form guidance in `SPEC.md`; a repo that mixes `INFRA-441` and `#441` for the same ticket will find neither with a query for the other, silently, no error), `--touches` (path match against `touches` on whole segments, in both directions: `--touches src/auth` finds commits listing `src/auth`, `src/auth/session.ts`, or the broader `src`, never `src/authz`), `--since`/`--until`, `agent-commit show ` for a single commit, both with a pretty-printed default view and a `--json` mode for machine consumption. Every result carries its commit hash (`hash` and `shortHash` in `--json`, the short hash leading each entry in pretty output), so anything found can be handed straight to `git show`. `agent-commit blame [-L [,]]` is the blame-time lookup described under "The problem" in one step: it runs `git blame` on those lines and prints the `Agent-Context` of each commit responsible, with the line ranges each one owns. No `--type` filter, on purpose, `type` isn't a field this format uses (see SPEC.md). `agent-commit check` reports, read-only, whether recent commits on the current branch have a usable `Agent-Context` trailer, distinguishing a trailer that was never written from one that exists in the message but wasn't recognized as a trailer (`MISPLACED`, see SPEC.md's squash-merge section for why that happens), useful right after a squash-merge to confirm the merge commit actually got one. `agent-commit merge-draft ` deterministically unions a branch's `touches`/`refs`/`breaking` and lists per-commit `why` candidates for a squash-merge trailer. `agent-commit init` writes the `AGENTS.md` block (see "How to use it" below) instead of an agent hand-transcribing it. `agent-commit --version` prints the installed version. Try it now with no install: `npx agent-commit log`. Building from source instead: `cd cli && npm install && npm run build && node dist/index.js log`. ## What's not built diff --git a/cli/README.md b/cli/README.md index 5dc6d54..76d7978 100644 --- a/cli/README.md +++ b/cli/README.md @@ -49,10 +49,17 @@ and the broader `src`, but never `src/authz`. `agent-commit show [--json]` Shows a single commit's `Agent-Context` trailer. -Every result from `log`, `show`, and `check` includes the commit's hash: +Every result from `log`, `show`, `blame`, and `check` includes the commit's hash: `hash` (full) and `shortHash` (git's own abbreviation) in `--json` output, and the short hash at the start of each entry in text output. +`agent-commit blame [-L [,]] [--json]` +Shows the `Agent-Context` of each commit that last changed the given +lines (the whole file without `-L`), via `git blame`, with the line +ranges each commit owns. `-L` takes git's numeric form, 1-based and +inclusive: `-L 42` or `-L 40,60`. Lines not committed yet are counted +on stderr rather than attributed to anything. + `agent-commit check [--range ] [--count ] [--json]` Read-only, advisory report on whether recent commits have a usable trailer. Distinguishes a commit with no trailer at all from one where an diff --git a/cli/src/blame.ts b/cli/src/blame.ts new file mode 100644 index 0000000..036501d --- /dev/null +++ b/cli/src/blame.ts @@ -0,0 +1,139 @@ +import { formatPretty } from "./format.js"; +import { parseLog, runGit, runGitLog } from "./parser.js"; +import type { AgentCommitRecord, ParseWarning } from "./types.js"; + +/** An inclusive range of line numbers in the blamed file, as it currently stands. */ +export type LineRange = [start: number, end: number]; + +export interface BlameEntry { + /** Full hash of the commit that last changed these lines. */ + hash: string; + /** Every line attributed to this commit, compressed into ranges, in file order. */ + lines: LineRange[]; +} + +export interface BlameParseResult { + /** One entry per distinct commit, ordered by where its first line appears in the file. */ + entries: BlameEntry[]; + /** Lines changed in the working tree but not committed yet; git attributes them to an all-zero hash. */ + uncommittedLines: number; +} + +export interface BlameRecord extends AgentCommitRecord { + lines: LineRange[]; +} + +export interface BlameResult { + records: BlameRecord[]; + uncommittedLines: number; + warnings: ParseWarning[]; +} + +/** + * Header lines in `git blame --porcelain` output: the commit hash, the + * line number in that commit's version, the line number in the file as it + * stands now, and (on the first line of a group only) the group's size. + * Verified against real git output. Every other line is either commit + * metadata starting with a lowercase keyword (`author`, `summary`, + * `previous`, `boundary`, ...) or the line's own content prefixed with a + * tab, so neither can be mistaken for a header. Accepts SHA-1 and SHA-256 + * object names. + */ +const PORCELAIN_HEADER = /^([0-9a-f]{40}|[0-9a-f]{64}) \d+ (\d+)(?: \d+)?$/; +const UNCOMMITTED_HASH = /^0+$/; + +function toRanges(lineNumbers: number[]): LineRange[] { + const sorted = [...lineNumbers].sort((a, b) => a - b); + const ranges: LineRange[] = []; + for (const line of sorted) { + const last = ranges[ranges.length - 1]; + if (last && line === last[1] + 1) { + last[1] = line; + } else { + ranges.push([line, line]); + } + } + return ranges; +} + +/** Parses `git blame --porcelain` output into the distinct commits responsible for the blamed lines. */ +export function parseBlamePorcelain(raw: string): BlameParseResult { + const linesByHash = new Map(); + let uncommittedLines = 0; + + for (const line of raw.split("\n")) { + const match = PORCELAIN_HEADER.exec(line); + if (!match) continue; + const [, hash, finalLine] = match; + if (UNCOMMITTED_HASH.test(hash)) { + uncommittedLines += 1; + continue; + } + const existing = linesByHash.get(hash); + if (existing) { + existing.push(Number(finalLine)); + } else { + linesByHash.set(hash, [Number(finalLine)]); + } + } + + const entries = [...linesByHash].map(([hash, lineNumbers]) => ({ hash, lines: toRanges(lineNumbers) })); + entries.sort((a, b) => a.lines[0][0] - b.lines[0][0]); + return { entries, uncommittedLines }; +} + +/** + * Parses a `-L` value: `` or `,`, 1-based and + * inclusive, the numeric subset of git blame's own `-L` syntax. git's + * regex and function-name forms are deliberately not accepted, so every + * value that reaches git here is known to be plain numbers. + */ +export function parseLineRange(value: string): LineRange | undefined { + const match = /^(\d+)(?:,(\d+))?$/.exec(value.trim()); + if (!match) return undefined; + const start = Number(match[1]); + const end = match[2] === undefined ? start : Number(match[2]); + if (start < 1 || end < start) return undefined; + return [start, end]; +} + +/** + * Finds the commits that last changed the given lines of a file (the + * whole file if no range is given), then reads each one's Agent-Context. + * Blames the working tree, like plain `git blame`, so lines not committed + * yet are counted rather than attributed to anything. + */ +export function blame(file: string, range?: LineRange, cwd?: string): BlameResult { + const blameArgs = ["blame", "--porcelain"]; + if (range) blameArgs.push("-L", `${range[0]},${range[1]}`); + // `--` ends option parsing, so a path starting with "-" is read as a path. + blameArgs.push("--", file); + + const { entries, uncommittedLines } = parseBlamePorcelain(runGit(blameArgs, cwd)); + if (entries.length === 0) return { records: [], uncommittedLines, warnings: [] }; + + // --no-walk=unsorted reads exactly these commits, in the order given. + const { records, warnings } = parseLog( + runGitLog(["--no-walk=unsorted", ...entries.map((entry) => entry.hash)], cwd), + ); + const linesByHash = new Map(entries.map((entry) => [entry.hash, entry.lines])); + return { + records: records.map((record) => ({ ...record, lines: linesByHash.get(record.hash) ?? [] })), + uncommittedLines, + warnings, + }; +} + +function formatRanges(ranges: LineRange[]): string { + return ranges.map(([start, end]) => (start === end ? `${start}` : `${start}-${end}`)).join(", "); +} + +/** Renders blame results like `log`'s pretty output, with each commit's line ranges right under its subject. */ +export function formatBlamePretty(records: BlameRecord[]): string { + return records + .map((record) => { + const [head, ...rest] = formatPretty(record).split("\n"); + return [head, ` lines: ${formatRanges(record.lines)}`, ...rest].join("\n"); + }) + .join("\n\n"); +} diff --git a/cli/src/index.ts b/cli/src/index.ts index 67f32ce..2ada10e 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -2,6 +2,7 @@ import { existsSync, readFileSync, statSync } from "node:fs"; import { fileURLToPath } from "node:url"; import { Command } from "commander"; +import { blame, formatBlamePretty, parseLineRange } from "./blame.js"; import { checkRecords } from "./check.js"; import { formatJson, formatPrettyList, sanitizeForTerminal } from "./format.js"; import { formatInitResult, needsAction, runInit } from "./init.js"; @@ -148,6 +149,37 @@ program process.stdout.write((opts.json ? formatJson(records) : formatPrettyList(records)) + "\n"); }); +program + .command("blame") + .argument("", "a file in the repository, as it currently stands in the working tree") + .description( + "shows the Agent-Context of each commit that last changed the given lines (the whole file " + + "without -L), via git blame. One entry per commit, ordered by where its lines first " + + "appear in the file. Lines not committed yet are counted on stderr, not attributed.", + ) + .option("-L ", "only these lines, 1-based and inclusive, e.g. -L 42 or -L 40,60") + .option("--json", "output machine-readable JSON instead of pretty text") + .action((file: string, opts: { L?: string; json?: boolean }) => { + let range: ReturnType; + if (opts.L !== undefined) { + range = parseLineRange(opts.L); + if (!range) { + fail(`-L must be or , with 1 <= start <= end, got "${opts.L}"`); + } + } + runFatal(() => checkGitVersion()); + + const { records, uncommittedLines, warnings } = runFatal(() => blame(file, range)); + printWarnings(warnings); + if (uncommittedLines > 0) { + process.stderr.write( + `agent-commit: ${uncommittedLines} line(s) not committed yet, so no commit to show for them\n`, + ); + } + + process.stdout.write((opts.json ? formatJson(records) : formatBlamePretty(records)) + "\n"); + }); + program .command("check") .description( diff --git a/cli/src/parser.ts b/cli/src/parser.ts index 0a7fa1f..e0771d6 100644 --- a/cli/src/parser.ts +++ b/cli/src/parser.ts @@ -129,8 +129,18 @@ export function checkGitVersion(cwd?: string): void { /** Runs `git log` with LOG_FORMAT, the NUL-delimited format parseLog expects. Extra args are passed through (revision ranges, --since, path filters, etc). */ export function runGitLog(extraArgs: string[] = [], cwd?: string): string { + return runGit(["log", `--format=${LOG_FORMAT}`, ...extraArgs], cwd); +} + +/** + * Runs any git subcommand and returns its stdout, translating failures + * into this module's typed errors (git missing, not a repository, or + * git's own non-zero exit with its stderr preserved) rather than leaking + * a raw execFileSync stack trace. + */ +export function runGit(args: string[], cwd?: string): string { try { - return execFileSync("git", ["log", `--format=${LOG_FORMAT}`, ...extraArgs], { + return execFileSync("git", args, { cwd, encoding: "utf8", maxBuffer: 1024 * 1024 * 64, diff --git a/cli/tests/blame.test.ts b/cli/tests/blame.test.ts new file mode 100644 index 0000000..d757e18 --- /dev/null +++ b/cli/tests/blame.test.ts @@ -0,0 +1,147 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { blame, formatBlamePretty, parseBlamePorcelain, parseLineRange } from "../src/blame.js"; +import { GitCommandError } from "../src/parser.js"; +import { record } from "./fixtures/record.js"; +import { TempRepo } from "./fixtures/temp-repo.js"; + +const A = "a".repeat(40); +const B = "b".repeat(40); +const ZERO = "0".repeat(40); + +describe("parseBlamePorcelain", () => { + it("groups lines by commit, compresses them into ranges, and orders commits by first line", () => { + // Shape copied from real `git blame --porcelain` output: a header per + // line (with a group size only on a group's first line), metadata once + // per commit, and tab-prefixed content. + const raw = [ + `${B} 1 1 2`, + "author t", + "summary second", + "filename f.txt", + "\tline one", + `${B} 2 2`, + "\tline two", + `${A} 3 3 1`, + "author t", + "summary first", + "boundary", + "filename f.txt", + "\tline three", + `${B} 5 4 1`, + "\tline four", + ].join("\n"); + expect(parseBlamePorcelain(raw)).toEqual({ + entries: [ + { hash: B, lines: [[1, 2], [4, 4]] }, + { hash: A, lines: [[3, 3]] }, + ], + uncommittedLines: 0, + }); + }); + + it("counts uncommitted lines (the all-zero hash) instead of attributing them", () => { + const raw = [`${A} 1 1 1`, "\tkept", `${ZERO} 2 2 2`, "\tnew", `${ZERO} 3 3`, "\tnew too"].join("\n"); + expect(parseBlamePorcelain(raw)).toEqual({ entries: [{ hash: A, lines: [[1, 1]] }], uncommittedLines: 2 }); + }); + + it("never mistakes line content or metadata for a header, even when it looks like one", () => { + const raw = [ + `${A} 1 1 1`, + `summary ${B} 1 1`, + `previous ${B} f.txt`, + `\t${B} 9 9 9`, + ].join("\n"); + expect(parseBlamePorcelain(raw).entries).toEqual([{ hash: A, lines: [[1, 1]] }]); + }); + + it("accepts SHA-256 object names", () => { + const sha256 = "c".repeat(64); + expect(parseBlamePorcelain(`${sha256} 1 1 1\n\tx`).entries).toEqual([{ hash: sha256, lines: [[1, 1]] }]); + }); +}); + +describe("parseLineRange", () => { + it("accepts a single line or an inclusive start,end pair", () => { + expect(parseLineRange("42")).toEqual([42, 42]); + expect(parseLineRange("40,60")).toEqual([40, 60]); + expect(parseLineRange("7,7")).toEqual([7, 7]); + }); + + it("rejects zero, reversed ranges, and git's non-numeric -L forms", () => { + for (const bad of ["0", "5,2", "abc", "", "1,", ",5", "-3", "1,+5", "/regex/", ":funcname"]) { + expect(parseLineRange(bad)).toBeUndefined(); + } + }); +}); + +describe("formatBlamePretty", () => { + it("puts each commit's line ranges directly under its subject", () => { + const out = formatBlamePretty([ + { + ...record({ shortHash: "aaaaaaa", subject: "fix: a", context: { why: "because" } }), + lines: [[1, 3], [7, 7]], + }, + { ...record({ shortHash: "bbbbbbb", subject: "chore: b" }), lines: [[4, 4]] }, + ]); + expect(out).toBe("aaaaaaa fix: a\n lines: 1-3, 7\n why: because\n\nbbbbbbb chore: b\n lines: 4"); + }); +}); + +describe("blame, against a real temp git repo", () => { + let repo: TempRepo; + afterEach(() => repo?.cleanup()); + + function seed(): void { + repo = new TempRepo(); + repo.commitFile("f.txt", "a\nb\nc\n", 'feat: first\n\nAgent-Context: {"why":"original reason"}'); + repo.commitFile("f.txt", "a\nB\nc\nd\n", 'fix: second\n\nAgent-Context: {"why":"second reason"}'); + } + + it("returns the commits behind the requested lines with their trailers and line ranges", () => { + seed(); + const { records, uncommittedLines, warnings } = blame("f.txt", [2, 3], repo.dir); + expect(records.map((r) => [r.subject, r.context, r.lines])).toEqual([ + ["fix: second", { why: "second reason" }, [[2, 2]]], + ["feat: first", { why: "original reason" }, [[3, 3]]], + ]); + expect(records[0].hash).toMatch(/^[0-9a-f]{40}$/); + expect(uncommittedLines).toBe(0); + expect(warnings).toHaveLength(0); + }); + + it("blames the whole file when no range is given", () => { + seed(); + const { records } = blame("f.txt", undefined, repo.dir); + expect(records.map((r) => [r.subject, r.lines])).toEqual([ + ["feat: first", [[1, 1], [3, 3]]], + ["fix: second", [[2, 2], [4, 4]]], + ]); + }); + + it("counts working-tree changes as uncommitted rather than attributing them", () => { + seed(); + repo.writeFile("f.txt", "a\nB\nc\nd\nnew\n"); + const { records, uncommittedLines } = blame("f.txt", [4, 5], repo.dir); + expect(records.map((r) => r.subject)).toEqual(["fix: second"]); + expect(uncommittedLines).toBe(1); + }); + + it("returns no records, without running git log, when every line is uncommitted", () => { + seed(); + repo.writeFile("f.txt", "a\nB\nc\nd\nnew\n"); + expect(blame("f.txt", [5, 5], repo.dir)).toEqual({ records: [], uncommittedLines: 1, warnings: [] }); + }); + + it("surfaces git's own error for a range past the end of the file or an unknown path", () => { + seed(); + expect(() => blame("f.txt", [9, 9], repo.dir)).toThrow(GitCommandError); + expect(() => blame("f.txt", [9, 9], repo.dir)).toThrow(/has only 4 lines/); + expect(() => blame("missing.txt", undefined, repo.dir)).toThrow(/no such path/); + }); + + it("treats a path starting with - as a path, never as a git option", () => { + repo = new TempRepo(); + repo.commitFile("-L", "x\n", "chore: oddly named file"); + expect(blame("-L", undefined, repo.dir).records.map((r) => r.subject)).toEqual(["chore: oddly named file"]); + }); +}); diff --git a/cli/tests/cli.test.ts b/cli/tests/cli.test.ts index 70c1cac..2aa4760 100644 --- a/cli/tests/cli.test.ts +++ b/cli/tests/cli.test.ts @@ -26,6 +26,41 @@ function runCli(args: string[], cwd: string) { return { stdout: result.stdout, stderr: result.stderr, status: result.status }; } +describe("agent-commit blame, against the compiled binary", () => { + let repo: TempRepo; + afterEach(() => repo?.cleanup()); + + it("prints each commit's lines and trailer, and notes uncommitted lines on stderr only", () => { + repo = new TempRepo(); + repo.commitFile("f.txt", "a\nb\n", 'feat: first\n\nAgent-Context: {"why":"the reason"}'); + repo.writeFile("f.txt", "a\nb\nnew\n"); + const { stdout, stderr, status } = runCli(["blame", "f.txt", "-L", "1,3"], repo.dir); + expect(status).toBe(0); + expect(stdout).toMatch(/^[0-9a-f]{7,} feat: first\n lines: 1-2\n why: the reason\n$/); + expect(stderr).toContain("1 line(s) not committed yet"); + }); + + it("emits records with a lines field under --json", () => { + repo = new TempRepo(); + repo.commitFile("f.txt", "a\n", "chore: one"); + const { stdout, status } = runCli(["blame", "f.txt", "--json"], repo.dir); + expect(status).toBe(0); + expect(JSON.parse(stdout)).toMatchObject([{ subject: "chore: one", lines: [[1, 1]] }]); + }); + + it("rejects a malformed -L before calling git, and reports git's own errors cleanly", () => { + repo = new TempRepo(); + repo.commitFile("f.txt", "a\n", "chore: one"); + const bad = runCli(["blame", "f.txt", "-L", "5,2"], repo.dir); + expect(bad.status).toBe(1); + expect(bad.stderr).toContain("-L must be or ,"); + const missing = runCli(["blame", "nope.txt"], repo.dir); + expect(missing.status).toBe(1); + expect(missing.stderr).toMatch(/^agent-commit: fatal: no such path/); + expect(missing.stdout).toBe(""); + }); +}); + describe("agent-commit log, against the compiled binary", () => { let repo: TempRepo; afterEach(() => repo?.cleanup()); diff --git a/cli/tests/fixtures/temp-repo.ts b/cli/tests/fixtures/temp-repo.ts index 49dcfb3..20195d5 100644 --- a/cli/tests/fixtures/temp-repo.ts +++ b/cli/tests/fixtures/temp-repo.ts @@ -32,6 +32,18 @@ export class TempRepo { }); } + /** Writes `content` to `path` (relative to the repo) and commits it with `message`, for tests that need a file's lines to change across commits. */ + commitFile(path: string, content: string, message: string): void { + writeFileSync(join(this.dir, path), content); + this.git(["add", "--", path]); + execFileSync("git", ["commit", "-q", "-F", "-"], { cwd: this.dir, input: message, encoding: "utf8" }); + } + + /** Writes `content` to `path` without committing, to leave a working-tree change. */ + writeFile(path: string, content: string): void { + writeFileSync(join(this.dir, path), content); + } + log(extraArgs: string[], format: string): string { return this.git(["log", `--format=${format}`, ...extraArgs]); }