:bug: Skip branch push when a run produces no commits by ibolton336 · Pull Request #114 · konveyor/agentic-controller

ibolton336 · 2026-08-07T13:36:10Z

The harness pushed the target branch ref unconditionally on exit, so no-op, refused, and skill-less runs each left an empty branch on the real repository — observed live as a stray konveyor/migration-… branch on konveyor/analyzer-lsp, and about to be multiplied by bulk workflow fan-out (one run per application).

Push now takes the base SHA captured immediately after clone/checkout and skips — with an explicit no commits produced; skipping push of <branch> log line — when HEAD still equals it. An empty base fails open: an unknown base must never block a push of real work. The watcher push path is covered too; it fires on filesystem events even with zero commits, so it was a second litter vector.

Deliberate caveats:

Runs with skills still push the harness's grounding-data commit (the base is captured before it), so this fully covers the skill-less/no-op case; the grounding commit itself is 🐛 Skip grounding-data commit when no analysis is returned #112's territory and the entry-point-slimming discussion's.
The "results pushed to branch" notice can overstate when the push was skipped; making it honest belongs to the planned run-outcome work, not this fix.

Tests: skip-on-no-commits (remote branch creation proven before the fix), push-with-commits byte-identical, and the workflow-stage case (a later stage adding no new commits skips; remote tip asserted unchanged). Harness module green under -race.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Empty branch pushes are now skipped when a run or workflow stage produces no new commits.
- Existing remote branches are preserved during no-op runs.
- Run notifications now accurately indicate whether changes were pushed, including cancelled runs.
Tests
- Added coverage for no-op pushes, new commit pushes, and staged runs without changes.
Documentation
- Added a changelog entry describing the updated push behavior.

coderabbitai · 2026-08-07T13:36:20Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 224fabfe-8154-483d-b017-2638b9790a54

📥 Commits

Reviewing files that changed from the base of the PR and between f523002 and 06e1ab7.

📒 Files selected for processing (4)

changes/unreleased/skip-empty-push.yaml
harness/cmd/migration-harness/main.go
harness/internal/git/git.go
harness/internal/git/git_test.go

📝 Walkthrough

Walkthrough

The harness now records the stage’s initial commit, skips pushes when no new commits exist, returns push status, and reports no-op or partial outcomes. Git tests cover lifecycle, new-commit, empty-push, and staged-push behavior. A changelog entry documents the fix.

Changes

Empty push handling

Layer / File(s)	Summary
Git push baseline and result contract `harness/internal/git/git.go`	Adds `HeadSHA` and updates `Push` to accept a baseline SHA, skip unchanged repositories, and return whether a push occurred.
Harness push wiring and notices `harness/cmd/migration-harness/main.go`	Passes the stage baseline to automatic and final pushes. Reports pushed, partial, cancelled, and no-op outcomes.
Push behavior tests and changelog `harness/internal/git/git_test.go`, `changes/unreleased/skip-empty-push.yaml`	Tests empty pushes, new commits, lifecycle integration, and staged pushes. Documents the updated no-change notices.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

konveyor/agentic-controller#86: Modifies stage-aware push and finalization behavior in harness/cmd/migration-harness/main.go.

Suggested reviewers: savitharaghunathan

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

savitharaghunathan · 2026-08-10T17:41:25Z

+func HeadSHA(repo *gogit.Repository) (string, error) {
+	head, err := repo.Head()
+	if err != nil {
+		return "", fmt.Errorf("resolve HEAD: %w", err)


Nit: this check looks redundant with the NoErrAlreadyUpToDate swallow below, but it's actually the only guard that prevents empty branch creation on the remote. When the branch doesn't exist yet, go-git's PushContext generates a push command (ZeroHash → local HEAD), so NoErrAlreadyUpToDate never fires.

A short comment above the if baseSHA != "" block explaining this distinction would prevent a future maintainer from removing it as belt-and-suspenders.

Good call — comment added in b7c7c7e capturing exactly that: on a missing remote branch PushContext sends a create command (ZeroHash → HEAD), so NoErrAlreadyUpToDate never fires and this guard is the only thing preventing empty branch creation.

savitharaghunathan · 2026-08-10T17:43:08Z

Misleading status notices on no-op runs

When Push skips because HEAD == baseSHA, the emitNotice and emitPush calls still report "results pushed to branch" / mark the push tool_call as completed. This could mislead operators and ACP tee viewers into thinking the branch has new content.

Consider having Push return a boolean indicating whether it actually pushed, so the caller can conditionally emit the notice — e.g. "stage succeeded — no changes to push" vs "results pushed to branch X".

Review follow-ups from konveyor#114: Push now returns whether it actually pushed, and the run notices use it so a skipped push reads "no changes to push" instead of claiming results landed on the branch. The failure/cancel notices only mention partial work when a push happened. A comment above the baseSHA guard records why it is not redundant with the NoErrAlreadyUpToDate swallow: on a missing remote branch PushContext sends a create command, so that error never fires and the guard is the only thing preventing empty branch creation. Co-Authored-By: Claude Fable 5 <[email protected]> Signed-off-by: ibolton336 <[email protected]>

ibolton336 · 2026-08-12T16:49:16Z

Fixed in b7c7c7e — Push now returns whether it actually pushed, and the notices use it: a skipped push reads "stage succeeded — no changes to push", and the failed/cancelled notices only claim partial work landed when a push happened. Bool asserted in all three push tests.

savitharaghunathan

Clean, well-scoped bugfix. The approach is sound:

Base SHA captured immediately after clone/checkout, before any harness work
Empty baseSHA (from HeadSHA failure) fails open — never blocks a real push
The guard at git.go:205-210 correctly handles the case where the remote branch doesn't exist yet (go-git sends a create command rather than reporting up-to-date, so NoErrAlreadyUpToDate alone wouldn't prevent empty branch creation)
Exit notices now branch on the pushed bool so messages are honest about what happened

Tests cover the three key scenarios: no-op skip, normal push, and multi-stage workflow where a later stage adds no new commits.

No bugs found. LGTM.

The harness pushed the target branch ref unconditionally on exit, so no-op, refused, and skill-less runs littered real repositories with one empty branch per run — multiplied by bulk workflow fan-out. Push now compares HEAD against the SHA captured after clone/checkout and skips, with an explicit log line, when the run produced nothing. Signed-off-by: ibolton336 <[email protected]> Co-Authored-By: Claude Fable 5 <[email protected]>

Review follow-ups from konveyor#114: Push now returns whether it actually pushed, and the run notices use it so a skipped push reads "no changes to push" instead of claiming results landed on the branch. The failure/cancel notices only mention partial work when a push happened. A comment above the baseSHA guard records why it is not redundant with the NoErrAlreadyUpToDate swallow: on a missing remote branch PushContext sends a create command, so that error never fires and the guard is the only thing preventing empty branch creation. Co-Authored-By: Claude Fable 5 <[email protected]> Signed-off-by: ibolton336 <[email protected]>

ibolton336 mentioned this pull request Aug 7, 2026

Run outcomes: refusal and no-op must not report bare Succeeded #129

Open

savitharaghunathan reviewed Aug 10, 2026

View reviewed changes

savitharaghunathan approved these changes Aug 12, 2026

View reviewed changes

ibolton336 and others added 2 commits August 12, 2026 17:51

ibolton336 force-pushed the fix/skip-empty-push branch from b7c7c7e to 06e1ab7 Compare August 12, 2026 21:52

ibolton336 merged commit a1cc6ee into konveyor:main Aug 12, 2026
12 of 13 checks passed

Navigation Menu

Uh oh!

🐛 Skip branch push when a run produces no commits - #114

🐛 Skip branch push when a run produces no commits#114
ibolton336 merged 2 commits into
konveyor:mainfrom
ibolton336:fix/skip-empty-push

ibolton336 commented Aug 7, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 7, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Uh oh!

savitharaghunathan Aug 10, 2026

Uh oh!

ibolton336 Aug 12, 2026

Uh oh!

savitharaghunathan commented Aug 10, 2026

Uh oh!

ibolton336 commented Aug 12, 2026

Uh oh!

savitharaghunathan left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ibolton336 commented Aug 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Uh oh!

savitharaghunathan Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

ibolton336 Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

savitharaghunathan commented Aug 10, 2026

Uh oh!

ibolton336 commented Aug 12, 2026

Uh oh!

savitharaghunathan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ibolton336 commented Aug 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 7, 2026 •

edited

Loading