Skip to content

fix(git): normalize log hashes for graph and revision consumers - #667

Open
HaningZS wants to merge 1 commit into
HarnessMD:mainfrom
HaningZS:fix/git-log-revision-whitespace
Open

HaningZS wants to merge 1 commit into
HarnessMD:mainfrom
HaningZS:fix/git-log-revision-whitespace

Conversation

@HaningZS

@HaningZS HaningZS commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

What & why

Git's --pretty=format: output inserts a newline between records. getLog() leaves it on the SHA of every row after the first, corrupting short hashes, making the agent Git tab's graph fail to match parents, and causing the returned ids to be rejected by the revision APIs.

Normalize only the full/short SHA fields, matching the existing getLogGraph() behavior. Other metadata, log ordering, and revision validation are unchanged. A real three-commit linear history now uses lanes [0, 0, 0] instead of [0, 1, 2], and every returned id can open its commit files and historical content.

Closes #666

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Docs
  • Build / CI

Evidence

Actual node:test output rendered as images. The same tests use real Git processes/repositories, the production log/revision functions, and the renderer's production graph layout. These are test-output images, not GUI screenshots.

Before

Unchanged upstream f44b986a fails all 8 new tests, including canonical ids, graph lanes, and use of non-first-row ids in revision APIs.

Before: newline-prefixed ids break the history contract

After

All 8 pass. Merge parents, tags, author/subject metadata, limited results and empty results are also covered.

After: canonical ids and correct graph parent matching

How I tested it

  • macOS 26.4.1, Apple Silicon, Node.js v24.13.0, Git 2.52.0.
  • node --test test/git-log-revisions.test.cjs test/commit-graph.test.cjs: 15 passed (8 new regressions).
  • npm run typecheck, npm run build, git diff --check: passed.
  • npm run test:focused: 971 passed, 1 failed, 1 skipped. Clean upstream at the same revision: 963 passed, the same 1 failure, 1 skipped. Both fail test/model-catalog-remote.test.cjs:60, catalog drift already tracked by CI never runs the test suite; model-catalog-remote is failing on main #652. The full-suite checkbox is left unchecked.

Agent review

Reviewed the active consumer (GitTab in agent details), graph parent matching, compatibility with getCommitFiles/getFileAtRev, paths with spaces, and metadata preservation. The production change is two field normalizations; it adds no process, dependency, UI style or permissive revision rule. The tests run real Git in temporary directories with spaces, disable fixture hooks/signing, and compare to actual Git ids rather than hardcoded hashes. The IDE History path already uses getLogGraph() and is unaffected.

Native Windows/Linux application smoke tests and GUI screenshots were not run; the tests demonstrate the actual data and layout failures directly. This does not redesign the log record format or address unrelated parser concerns.

Checklist

  • Before and after evidence is attached under both headings.
  • npm run typecheck passes.
  • npm run test:focused passes — the independently reproduced upstream failure is detailed above.
  • npm run build succeeds.
  • This PR is one change.
  • I read the diff for debug output and unrelated changes.
  • No new UI or art; design-token and artwork requirements are not applicable.

Why:
- Git inserts a newline between pretty=format records, so getLog returns invalid hashes after the first row and the agent Git graph cannot match parents.

What:
- Trim full and short hashes in getLog, matching the existing getLogGraph behavior.
- Add real Git integration tests for canonical ids, graph lanes, revision APIs, merges, decorations and limits.
- Attach rendered before/after test evidence.

Risk:
- Only the hash field is normalized; commit metadata and revision validation are unchanged.
- Native Windows/Linux application smoke tests were not run.

Tests:
- New suite: 8 passed; unchanged upstream: 8 failed.
- Related graph tests: 15 passed including the new suite.
- npm run typecheck; npm run build; git diff --check passed.
- Full suite: 971 passed, 1 failed, 1 skipped; baseline: 963 passed, same model-catalog failure (HarnessMD#652), 1 skipped.

Live Docs:
- No documentation behavior changed; required PR evidence under docs/pr-evidence.

This branch has not been deployed

No deployments
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.

Agent Git tab log ids contain leading newlines and break parent matching

1 participant