feat: add M5 git workflow backend#72
Conversation
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds a zerogit module to inspect/commit git changes, extends verify with output summarization and a bounded RunLoop, and wires both into the CLI: ChangesChanges inspection and verify loops
Sequence Diagram(s)sequenceDiagram
participant CLI as User CLI
participant ZeroGit as zerogit.Inspect/Commit
participant Verify as verify.RunLoop/Run
participant Git as Git Runner
alt changes inspect
CLI->>ZeroGit: Inspect(ctx, InspectOptions)
ZeroGit->>Git: git rev-parse, git status, git diff (GIT_INDEX_FILE)
Git->>ZeroGit: stdout/stderr
ZeroGit->>CLI: ChangeSummary (files, diff, truncated)
else changes commit
CLI->>ZeroGit: Commit(ctx, CommitOptions)
ZeroGit->>ZeroGit: Inspect() -> ChangeSummary
ZeroGit->>Git: git add -A
ZeroGit->>Git: git commit -m [message]
ZeroGit->>Git: git rev-parse --short HEAD
Git->>ZeroGit: stdout
ZeroGit->>CLI: CommitResult (committed, commitHash, before)
else verify --attempts N
CLI->>Verify: RunLoop(ctx, plan, MaxAttempts=N)
loop Each attempt
Verify->>Verify: Run(ctx, plan)
Verify->>Verify: Append Attempt to LoopReport
alt Pass
Verify->>CLI: LoopReport OK
else Fail & OnFailure
Verify->>Verify: Invoke OnFailure -> decide retry/stop
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/verify/verify.go`:
- Around line 274-276: The code sets truncated = true when len(lines) >= 8,
marking summaries truncated even if exactly eight lines were captured; change
the logic so Truncated is set only when content beyond the capture limit exists
(e.g., check for len(lines) > 8 or verify there is at least one additional
relevant line before setting truncated). Locate the block that checks len(lines)
>= 8 in verify.go and modify it to detect an actual overflow (peek the next line
index or use the total count) and only set truncated = true when there is more
content to drop.
- Around line 255-286: summarizeOutput currently returns nil when no lines match
isFailureLine, dropping plain execution errors; update summarizeOutput to fall
back to populating OutputSummary with the first non-empty trimmed line from the
provided values when no failure-context lines were captured (preserving existing
trimming, redaction behavior if present), still respecting the max-8-lines cap
and setting Truncated appropriately; keep the main loop and only create the
fallback OutputSummary (with Lines containing that one line) before returning
nil so that OutputSummary is never nil for simple command errors.
In `@internal/zerogit/zerogit.go`:
- Around line 297-306: The current branch that handles a redaction marker when
budget <= 0 can return strings longer than maxBytes (e.g. returning
redaction.RedactedSecret + suffix). Update that branch in the function using
variables suffix, marker, budget, head so the returned string never exceeds
maxBytes: if len(suffix) >= maxBytes return suffix[:maxBytes]; otherwise compute
allowed = maxBytes - len(suffix), take the first allowed bytes/characters of
redaction.RedactedSecret (or the marker) and append suffix, and return that;
this ensures all returns respect the MaxDiffBytes budget.
- Around line 88-92: Inspect() currently builds ChangeSummary.DiffStat/Diff by
calling gitRawOutput with "diff --stat HEAD --" and "diff HEAD --", which fails
for unborn HEAD and omits untracked files; change the logic to create a
temporary index that stages untracked files (use a temporary GIT_INDEX_FILE and
run "git add -A" into that index) and run "git diff --cached --stat --" and "git
diff --cached --" against that staged snapshot instead of HEAD, and add a
fallback that uses the empty-tree diff (no-HEAD) when HEAD is absent (detect via
"git rev-parse --verify HEAD" or error from git and then use the empty tree SHA)
so ChangeSummary.DiffStat/Diff reflect what "git add -A" + commit would produce;
update tests to cover unborn HEAD and untracked-only changes to assert preview
matches the commit outcome.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ba49d743-caf6-4365-8ea8-8855ec67d64a
📒 Files selected for processing (7)
internal/cli/app.gointernal/cli/workflow_test.gointernal/cli/workflows.gointernal/verify/verify.gointernal/verify/verify_test.gointernal/zerogit/zerogit.gointernal/zerogit/zerogit_test.go
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/zerogit/zerogit_test.go`:
- Around line 265-273: The test helper runGitCommand should use a context tied
to the test deadline so a stuck git process can be cancelled: replace
exec.Command(...) with exec.CommandContext(ctx, "git", args...) where ctx is
derived from t.Deadline() (e.g., context.WithDeadline or context.WithTimeout
using time.Until(t.Deadline())), ensure you import the context package (and time
if using time.Until), and propagate that ctx when creating the command; keep the
existing error handling and return value in runGitCommand.
In `@internal/zerogit/zerogit.go`:
- Line 282: The defer currently calls os.RemoveAll(tempDir) without handling its
returned error; make the ignored return explicit so errcheck is satisfied by
changing the defer to explicitly discard the error (e.g., use the blank
identifier) or handle/log it; update the defer that references tempDir in
zerogit.go (the defer os.RemoveAll(tempDir) call) so the intent to ignore the
error is explicit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b738f4c-283f-44f8-aaf0-13e44ec2db99
📒 Files selected for processing (4)
internal/verify/verify.gointernal/verify/verify_test.gointernal/zerogit/zerogit.gointernal/zerogit/zerogit_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/verify/verify.go
|
Looks Good I reviewed latest head Blockers
Non-Blocking
Looks Good
PR-branch validation passed:
Latest-main test-merge validation passed:
GitHub checks are green: Smoke ubuntu, Smoke macOS, Smoke Windows, Performance Smoke, Zero Review, and CodeRabbit. |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Looks Good
I reviewed latest head c04c58688815b2d1bd188230bd15d34e5a938237 for the M5 git workflow backend. The PR branch is based on an older main (a58e6c4... while current origin/main is 43c2560...), so I validated both the PR branch and a latest-main test merge. The merge applied cleanly and the targeted/full validation still passed there.
Blockers
- None.
Non-Blocking
- The branch is stale relative to current
main, but a detachedorigin/main+ PR-head test merge completed cleanly. A normal update/rebase before merge would make the final diff easier to read, but I do not see a code-review blocker from the stale base.
Looks Good
internal/zerogitkeeps inspect previews non-mutating by using an alternateGIT_INDEX_FILEfor the staged snapshot diff, while the real commit path intentionally stages withgit add -Aonly outside dry-run mode.- Change summaries redact status paths/diff/diffstat, truncate bounded diff output, and keep ordinary diagnostic fields useful.
zero changes inspectandzero changes commitroute JSON/text output through the CLI redaction layer before formatting.zero verify --attemptsexposes a bounded loop report while preserving the single-attempt behavior for the default path.- Runtime probes passed for:
./zero.exe changes inspect --cwd <temp-git-repo> --json --diff-bytes 600./zero.exe changes inspect --cwd <temp-git-repo> --diff-bytes 600./zero.exe changes commit --cwd <temp-git-repo> --dry-run --message "Update secret probe" --json./zero.exe changes commit --cwd <temp-git-repo> --dry-run --message "Update secret probe"- a real
./zero.exe changes commit --cwd <temp-git-repo> --message "Add secret probe" --json, verifying HEAD advanced and the worktree was clean afterward ./zero.exe verify --attempts 2 --only go.test --timeout-ms 600000 --json
- Redaction probes used assembled OpenAI-shaped dummy values and verified the exact raw value was absent from inspect and commit dry-run JSON/text output.
PR-branch validation passed:
go test -count=1 ./internal/zerogit ./internal/verify ./internal/cligo test -count=1 -p 1 ./...go build ./cmd/zerobun install --frozen-lockfilebun run typecheckbun test ./tests --timeout 15000— 291 pass / 0 failbun run buildbun run smoke:buildbun run smoke:go./zero.exe --help./zero.exe changes --help./zero.exe verify --helpgit diff --check $(git merge-base HEAD origin/main)..HEAD
Latest-main test-merge validation passed:
git merge --no-commit --no-ff origin/pr/72from currentorigin/maingo test -count=1 ./internal/zerogit ./internal/verify ./internal/cligo test -count=1 -p 1 ./...go build ./cmd/zerobun install --frozen-lockfilebun run typecheckbun test ./tests --timeout 15000— 217 pass / 0 failbun run buildbun run smoke:buildbun run smoke:go./zero.exe changes --help./zero.exe verify --helpgit diff --check --cached
GitHub checks are green: Smoke ubuntu, Smoke macOS, Smoke Windows, Performance Smoke, Zero Review, and CodeRabbit.
Summary
internal/zerogitbackend for local git change inspection, redacted diff summaries, generated commit messages, dry-run commit previews, and realgit add -A/ commit execution.zero changes inspect,zero changes commit, andzero verify --attemptswith JSON/text output and CLI regression coverage.Validation
go test ./internal/zerogit ./internal/verify ./internal/cligo test ./...go test -count=1 ./internal/zerogit ./internal/verify ./internal/cligo test -count=1 -p 1 ./...go build ./cmd/zerobun run typecheckbun test ./tests --timeout 15000- 291 pass / 0 failbun run buildbun run smoke:buildbun run smoke:go/tmp/zero-m5 changes inspect --json --diff-bytes 600/tmp/zero-m5 changes commit --dry-run --message "Update M5 git backend"/tmp/zero-m5 verify --attempts 2 --only go.test --timeout-ms 600000 --jsongit diff --check origin/main..HEADReviewers: @Vasanthdev2004 @anandh8x
Summary by CodeRabbit
New Features
Tests