Skip to content

feat(api): add GitTarget spec.parentBranch, the branch a new write branch starts from - #408

Merged
sunib merged 2 commits into
fix/new-branch-checks-parentfrom
feat/gittarget-parent-branch
Oct 1, 2026
Merged

sunib merged 2 commits into
fix/new-branch-checks-parentfrom
feat/gittarget-parent-branch

Conversation

@sunib

@sunib sunib commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Stacked on #407. Review that one first; this diff is against its branch.

What

GitTarget.spec.parentBranch names the branch the write branch (spec.branch) is created from. While the remote does not carry the write branch, the target compares against the parent. Omitting the field keeps today's behavior: the remote's default branch, and a branch with no history in an empty repository.

spec:
  branch: reverser/prod-edits   # the write branch (unchanged, immutable)
  parentBranch: env/prod        # optional; omitted = the remote's default branch

This is the standby flow #407 made correct, applied to release-1.x or env/prod, not just main. Once the write branch exists, the parent is not followed. That matches Flux ImageUpdateAutomation and Argo CD's hydrator. Resetting a merged write branch onto its parent is separate work.

Contract

Write branch on remote Parent on remote Behavior
yes any unchanged: work on the write branch
no yes compare against the parent; create the write branch from its tip, checked at publication (#407)
no no, set refuse: Ready=False / Stalled=True, reason ParentBranchNotFound, nothing written, no orphan branch
no no, omitted, empty repository unchanged: unborn HEAD, root commit
  • Mutable. It only says where a branch starts, so changing it abandons nothing. A change keeps the branch worker and its clone, and drops base trust, so the next cycle re-reads the new parent. It is not handled like a repoint.
  • Not in allowedBranches. It is only read.
  • parentBranch == branch is legal when that branch exists. When it is absent, creation is refused, including in an empty repository. Only an omitted parent bootstraps.
  • One parent per write branch. One worker owns one checkout, so checkForConflicts reports TargetConflict for two targets on the same provider and write branch that name different parents. The later one loses. Values are compared as written, so omitted and main conflict even when the remote's HEAD is main. The answer needs no round trip and does not flip when the remote's default branch changes.
  • CRD: MinLength=1 when set; optional; no transition rule.

How

  • SmartFetchFrom, PrepareBranchFrom and syncToRemoteFrom take the parent. A configured parent is the fallback when the target is absent; if the parent is missing too, they return ErrParentBranchNotFound instead of an empty ref. The remote-HEAD mirror is still written from the remote's real HEAD.
  • The worker holds the parent as an atomic, set by EnsureWorker, and passes it to every fetch through two wrappers. A fetch that hits ErrParentBranchNotFound records a remote observation with MissingParent set, because the advertisement did say so.
  • The refresher reads the configured parent from the same advertisement. It follows a parent that moved, and reports a missing one without fetching.
  • GitTargetReconciler.parentBranchReadiness reads that observation: no round trip, and it works with --git-refresh-interval=0. It stalls the target as ParentBranchNotFound. While that reason holds, each reconcile forces a recheck, the same recovery a refused Git path uses, so the target recovers once the branch is pushed.

Tests

  • internal/git:
    • SmartFetchFrom table: omitted, configured, missing, existing target with a missing parent, parent equal to the target (present and absent), and an empty repository.
    • Worker:
      • a configured parent that moves after the checkout;
      • a missing parent writes nothing, then recovers once it is pushed;
      • parent equal to branch (exists / absent / empty remote);
      • an existing write branch does not follow its parent;
      • SetParentBranch invalidates the base.
    • Refresher: follows a configured parent, including one changed on a live target, and reports a missing one at one connection with no fetch.
  • internal/controller:
    • conflict table (omitted versus main, earlier wins, same parent on sibling paths, another write branch);
    • a parent change keeps the worker;
    • the readiness gate;
    • an envtest case: the field can be set, changed and cleared, but not set to "".
  • e2e: the feat(api): add GitTarget spec.parentBranch, and create a new write branch on its parent's current tip #407 spec now runs twice: parent omitted, and spec.parentBranch: release. In the second run release is not in allowedBranches.

Docs

  • architecture.md: the GitTarget key fields, the conflict rule, and the new-branch paragraph now names the configured parent.
  • definitions.md: write branch and parent branch added under "Write side".
  • configuration.md: a settings-table row (via hack/crdfields/fields.yaml) and a "Starting the write branch from another branch" section.

Validation

task lint and task test pass, with coverage at baseline. The local e2e run is pending.

🤖 Generated with Claude Code

…anch starts from

spec.parentBranch names the branch the write branch is created from, and
compared against while the remote does not carry it. Omitted keeps the
remote's default branch. It is mutable and only read, so it need not be in
allowedBranches.

A configured parent must exist for the write branch to be created: a fetch
that finds neither reports ParentBranchNotFound and writes nothing, so a typo
never starts an orphan branch. Every GitTarget on one write branch must name
the same parent, compared as written; the later one reports TargetConflict.
A change keeps the branch worker and drops its base trust.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 9 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: ae76fade-3f52-46ed-96ef-8d17ce4ffea5

📥 Commits

Reviewing files that changed from the base of the PR and between 6ac4ba3 and 55e3eb5.

📒 Files selected for processing (34)
  • .coverage-baseline
  • api/v1alpha3/gittarget_types.go
  • config/crd/bases/configbutler.ai_gittargets.yaml
  • docs/architecture.md
  • docs/configuration.md
  • docs/definitions.md
  • hack/crdfields/fields.yaml
  • internal/controller/condition_helper.go
  • internal/controller/gittarget_controller.go
  • internal/controller/gittarget_immutability_test.go
  • internal/controller/gittarget_layout_test.go
  • internal/controller/gittarget_parent_branch_test.go
  • internal/controller/gittarget_status_test.go
  • internal/controller/gittarget_worker_cleanup_test.go
  • internal/git/branch_worker.go
  • internal/git/branch_worker_base_trust_test.go
  • internal/git/branch_worker_new_branch_test.go
  • internal/git/branch_worker_parent_branch_test.go
  • internal/git/branch_worker_queue_depth_test.go
  • internal/git/branch_worker_split_test.go
  • internal/git/git.go
  • internal/git/git_smart_fetch.go
  • internal/git/refresh.go
  • internal/git/refresh_test.go
  • internal/git/remote_observation.go
  • internal/git/remote_report_test.go
  • internal/git/replay_failure_test.go
  • internal/git/worker_manager.go
  • internal/git/worker_manager_test.go
  • internal/git/worker_repo_identity_test.go
  • internal/watch/event_router_test.go
  • test/e2e/helpers.go
  • test/e2e/new_branch_parent_e2e_test.go
  • test/e2e/templates/gittarget.tmpl
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.74436% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/git/worker_manager.go 50.0% 2 Missing ⚠️
internal/git/branch_worker.go 96.6% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@sunib
sunib added this pull request to stack #409 October 1, 2026 07:52
…arent checks

Review fixes for spec.parentBranch:

- A parent change now reaches writes retained during the push cooldown. They
  were planned against the old parent, and an invalidated base does not
  reach them, so the event loop takes the change and refetches and replays
  before the next commit or push. A missing new parent then refuses
  publication instead of creating the branch from the old one.
- Parent conflicts apply whatever either target's path is. A target with a
  path its writer rejects still wires the shared worker and would set its
  parent; path validity now gates only the overlap check.
- ParentBranchNotFound ignores an observation proved against another
  repository than the one the GitProvider names now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sunib
sunib merged commit 55e3eb5 into main Oct 1, 2026
8 checks passed
@sunib
sunib deleted the feat/gittarget-parent-branch branch October 1, 2026 08:14
@sunib

sunib commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Folded into #407, which now carries these commits (field b0fc1d1, review fixes 55e3eb5) on top of the fix and targets main. GitHub does not allow retargeting a stacked PR, so the combined PR is #407. Closing in favor of #407.

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.

1 participant