Repository navigation
Commit 2de6dc8
feat(api): add GitTarget spec.parentBranch, and create a new write branch on its parent's current tip (#407)
* test(git): reproduce a new write branch created on a stale parent
Pins today's behavior before the fix: a warm worker that trusts its checkout
of the parent creates the write branch on that checkout even after the
parent moved, a no-diff live write creates the write branch anyway, and a
branch somebody created concurrently is pushed over.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(git): check the parent before creating a write branch, and create none for a no-op
A push that creates the write branch is rooted on the parent it was built
from, but validatePushState compared that root only when the pushed branch
already existed. A warm worker that trusted its checkout of the parent
therefore created the branch on that checkout, however far the parent had
moved. The root is now compared either way, and a branch somebody created
since we based our work is reported as a moved remote instead of being
pushed over; the worker replays onto it.
A cycle that commits nothing no longer creates the write branch merely to
record that nothing changed: the push settles as NoBranch once the parent is
confirmed. The worker remembers the parent a local write branch was created
from, so a no-op cycle that leaves HEAD on that local branch still roots the
next cycle on the parent.
The refresher follows the parent of an absent write branch from the same
advertisement, so the folder it re-reads is the tree the next commit builds
on. Publication does not depend on it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* feat(api): add GitTarget spec.parentBranch, the branch a new write branch 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>
* fix(git): rebuild retained writes on a parent change, and scope the parent 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>
* feat(api): make spec.parentBranch immutable, and report status.remote.parent
spec.parentBranch decides what history a new write branch gets, so it is
fixed at creation like branch and path, including adding or removing it.
Delete and recreate loses nothing: the destination is unchanged. The worker
keeps its parent-change handling for the narrow case where every target on
a branch is replaced before the worker sweep.
While the write branch is absent, status.remote.parent names the parent it
would be created from and the commit it is at, observed together with the
branch's absence by every fetch, refresh and push. With spec.parentBranch
omitted, it shows which default branch the remote resolved; an empty commit
means the parent is missing too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* feat(api): report whether the parent was found, missing or unborn
status.remote.parent.state is Found (the first commit builds on the
parent's tip), Missing (spec.parentBranch is not on the remote either, so
nothing is written) or Unborn (an empty repository and no parent configured,
so the first commit has no history). Without it, a blocked target and a
fresh repository both showed an empty commit, or no parent at all.
The worker's observation carries the same state instead of a MissingParent
name, so the readiness gate and the status read one fact.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(git): keep a parent change pending until the rebuild succeeds
recoverRetainedWrites took the parent-change flag before the rebuild ran. A rebuild that failed
before its reset, reading the GitProvider or the credentials, left nothing remembering that the
retained writes sat on the old parent, so the retry published from it, or created the branch
although the new parent was missing. Hand the change back when the rebuild fails.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs: record where the parent-branch work goes next, and the open #404 follow-ups
Four design pages, checked in so the direction is visible:
- gittarget-parent-branch.md: the original plan, now implemented in this PR.
- gittarget-parent-hardening.md: the hardening pass from the review of this PR. It keeps the fast
path pull-free, and covers parent changes on a live worker, an empty repository that gains a ref,
an unresolved default branch, and recovery that cannot strand work.
- gittarget-parent-empty-repository.md: an empty repository's default branch. It assumes main
first; asking the server needs a small go-git change, already tried in an experiment.
- gittarget-parent-observation.md: keep the parent visible after the write branch exists.
todo.md lists the follow-ups from the #404 review.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs: keep the parent-branch pages to what is true, and set the merge boundary for #407
- gittarget-parent-branch.md now records the contract as built (immutable parentBranch,
status.remote.parent, the conflict rule) and the reasoning behind it. The executed plan, the
handoff, the architecture.md edits already applied, and the superseded "mutable" decision are
gone.
- gittarget-parent-hardening.md states the merge boundary, makes the CommitRequest timeout fix a
prerequisite, tracks recovery per (target, collection) until published, and keeps the fallback's
backoff per worker.
- gittarget-parent-empty-repository.md is deferred: get the server's evidence through go-git
first, then decide the bootstrap policy. Assuming main is recorded as considered and rejected.
- gittarget-parent-observation.md ships availability before ancestry, and authorizes no mutation.
- The #404 follow-ups move from a root todo.md into docs/TODO.md, by priority. INDEX lists the
four pages.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(commit): never fail a CommitRequest the worker still holds, and withdraw one before failing it
The controller failed a request closed attachTimeout + maxDuration + 120s after creation (about
124s with defaults) while the worker kept the attach. A remote down for two minutes, or a parent
branch recovery, therefore reported FinalizeFailed for a request the worker then committed anyway.
Controller and worker now agree:
- A request the worker holds (CollectingWindow or WaitingForPush) has no controller-side timeout;
only the worker ends it.
- An unheld request past the bound is withdrawn first. The withdraw rides the worker's FIFO behind
every attach, cancels a request that is only waiting for a window (or was never registered), and
leaves a tombstone so a late attach cannot register it again. For a request the worker holds it
is a no-op, and the controller keeps waiting.
- The controller fails the request only on the worker's answer, and reports the real outcome when
the worker resolved it first. A GitTarget that never started a worker is named in the message.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(commit): withdraw a CommitRequest from the worker that holds it, and settle requests when a worker exits
Review of 0474b34 found three ways a withdraw could report a request failed while a worker could
still publish it, or never answer:
- A missing GitTarget, or no manager entry, was read as "nothing holds the request". A worker is
shared by every GitTarget on its branch, so it outlives a deleted one, and a retired worker is
detached before it stops, so its last push runs unlisted. The manager now records which worker
accepted each request's attach (commitRequestOwners), and the router asks that worker first.
Only a request no worker holds falls back to the GitTarget.
- The withdrawn outcome could be collected by its 15m TTL while an attach for it was still queued,
and that attach then registered the request again. An outcome is now kept while any attach for
it is queued, and past the safety window the controller only withdraws, never attaches.
- A worker whose loop exited, including one that never started because its GitProvider could not
be read, stayed listed while attaches and withdraws queued unanswered. On exit a worker now
closes admission, drains its queue, gives back every request it never acted on, fails any it
held, and answers a withdraw at once.
The router regression test fails on the previous router with "the withdraw reported success while
the worker still held the registration".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(git): scope base trust to the parent generation and admit a push only on its current root
SetParentBranch ran on the controller goroutine, dropped trust once and raised a flag the event
loop could undo or bypass: a push or fetch finishing after the change set trust again, the
remembered new-branch parent then rooted the next write on the old parent, and a change landing
between the rebuild and the push was pushed on the old root.
- The parent configuration is one immutable {name, gen} snapshot. Every fetch loads it once and
records what it actually used; nothing re-reads the name after the network call.
- Trust is scoped to the generation the checkout was proven under, so a late setBaseTrusted(true)
cannot vouch for a newer configuration, and the remembered new-branch parent counts only under
the current one.
- runPushCycle checks, under repoMu right before each push, that the root was chosen under the
current generation; otherwise it replays first, with no extra connection. A push already
admitted is not recalled: a branch it creates from the old parent stays, and an existing branch
does not follow its parent.
- A cycle rooted on an existing write branch is re-stamped without a fetch, because a parent
cannot move it. SetParentBranch therefore no longer invalidates the base; the generation does.
- A failed rebuild leaves the generations apart, which replaces the flag hand-back of dcbc8b8.
Five of the seven new tests fail on the previous worker, including the review's no-op-push
reproduction; the other two pin the in-flight rule.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(git): never orphan a branch in a repository that is not empty
Two ways an omitted parent started the write branch with no history beside refs it ignored:
- A cycle planned on an empty repository (root = zero) pushed after somebody created main: no check
fired, because the missing-root check needs a non-zero root and the moved check compared the
absent write branch with zero. validatePushState now refuses with RepositoryNotEmptyError when
the root is zero, the write branch is not advertised and the advertisement carries other refs.
It reads the advertisement the push already holds, and remoteMovedDuringPush treats it as
contention, so the replay resolves the new default branch and re-plans from the retained writes.
- SmartFetchFrom returned the "empty repository" answer for a repository with refs but no
resolvable default branch (HEAD dangling, or only tags), and the caller made HEAD unborn. It now
returns DefaultBranchUnresolvedError, handled like ErrParentBranchNotFound on the fetch, the
replay and the refresh: no write is planned and the parent is observed Missing.
"Empty" has one definition: no hash refs at all, tags included. Default-branch resolution follows
go-git, which rewrites a hash-only HEAD before Remote.List returns; that is pinned by a test.
An omitted parent can now stall: the observation carries ParentRequested, and
parentBranchReadiness compares it with spec.parentBranch, with a message that says to fix HEAD or
set the field. Canonical git advertises neither a HEAD that resolves to nothing nor its symref, so
against it the message says the remote has no default branch.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(git): make recovery from a missing parent an obligation the worker drives
While a parent branch was missing, every reconcile forced a recheck (restarting every stream of the
target), indefinitely for a typo, and recovery still depended on an observation flip: the refresher
skips a branch holding work, a failed push stops the push timer, refresh can be off, and a first
attempt that fails transiently after the parent reappears stranded the work.
The worker now latches an obligation the moment a cycle fails on the parent, and clears it only when
the work is published:
- retained writes stay retained until pushPending publishes them;
- writes it had to drop are remembered per (GitTarget, collection) scope, and a scope clears only
when a resync of it is published (pushed, or settled as a no-op), not on the resync reply.
The worker drives it on its own deadline (10s, doubling, capped at 5m, one per worker): one
advertisement while the parent is missing; once found, it retries the retained writes and raises a
per-target SnapshotRequestSeq for the scopes still owed. Before the deadline a refresh tick costs
nothing and a live write or resync fails without a fetch; reconciles never probe. A parent change
makes the probe due at once.
The controller forces one recheck per rise of the sequence (remembered in memory, like reconcile
requests) instead of on every reconcile while the Ready reason is ParentBranchNotFound, and reports
Ready=False/Reconciling with reason RecoveringParentBranch while the parent is back but work is owed.
Ledger rows 1-12 are unchanged; new rows: standby no-op live write (1), standby first write after the
parent moved (6, as row 6), parent probe while missing (1 advertisement, 0 fetches).
e2e: a third context recovers from ParentBranchNotFound with no cluster edit after the parent is
pushed. The refresher-timing step ("contention rose, refresh did not") is removed: with refresh on, a
refresh can find B first; the worker tests prove publication alone finds a moved parent.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(git): follow a switched default branch, and replay onto a write branch created during our upload
R5. With spec.parentBranch omitted the parent is the default branch as last discovered, but the
refresher compared the parent's hash only: a HEAD switched from main to trunk at the same commit was
never followed, and no-op pushes kept recording main while refreshes recorded trunk. followParent now
also compares the advertised parent branch with the one the checkout is rooted on, and follows a
mismatch with one fetch. Publication still cannot see HEAD; that is documented.
R6. When somebody created the write branch between our advertisement and our upload, the server
refused the upload (Old was zero) as an untyped error, the fallback probed the root only, found the
parent unmoved, and the work never replayed. On that path only, the fallback now reads both refs
from one advertisement and treats a write branch that is now there as contention. Its ledger row
measures 8 connections: the refused upload, one advertisement, the contention fetch, the push.
Also rewords the "parent branch" vocabulary rule: a bare "parent" is fine where the subject is
already a branch, such as status.remote.parent.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs: fold the hardening pass into the parent-branch contract, and pin a held request through a long recovery
The contract page now carries what shipped: the omitted-parent refusal for a repository that is not
empty, recovery as a worker obligation, the push admission boundary, go-git's default-branch
resolution, and the new ledger rows. The hardening plan is marked done and records where the build
departed from it.
TestParentRecovery_AHeldCommitRequestIsCommittedAfterALongRecovery runs the real event loop: a
CommitRequest riding a write held back by a missing parent stays WaitingForPush through a recovery
far longer than the controller's old bound, and resolves Committed on the parent's tip.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs: describe the safety window as a withdrawal request, and fix the review nits
- api-first-publication.md and commit-timing-surface.md: past the safety window the controller
asks the worker to withdraw the request and reports the worker's answer, which can be the real
outcome when the worker resolved it first.
- configuration.md: the status.remote example shows an absent write branch, so the parent stanza
it shows is one the contract can produce.
- gittarget-parent-branch.md: drop the settled default-branch question from the open questions.
- INDEX.md headings and TODO.md wording follow the style guide.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(git): keep a recovery obligation for work dropped in a second outage, and hold retained replay to the probe deadline
Review of 93120ca reproduced two §4.4 defects:
- P1. A recovery snapshot that had committed but not yet been pushed kept its publication credit
when the parent vanished again and another write from the same scope was dropped. Publishing the
older snapshot then cleared the newer obligation: cm1 landed, cm2 never did, and no new snapshot
was requested. A dropped write now removes its scope's credit, so the scope stays open and the
worker asks again.
- P2. With retained writes waiting for a rebuild, every incoming write went through
recoverRetainedWrites, which fetched without checking the probe hold: one connection per write
before the deadline. It now returns the hold's error before any I/O, and the scheduled probe
still drives the recovery.
The reviewer's reproductions are the regression tests (both red before the fix); the probe-budget
test gains a retained-writes case. The docs record the review, its resolution, and the follow-up
order after #407 merges.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>1 parent 7f090ea commit 2de6dc8
64 files changed
Lines changed: 7326 additions & 275 deletions
File tree
- api/v1alpha3
- config/crd/bases
- docs
- design
- spec
- hack/crdfields
- internal
- controller
- git
- testdata
- watch
- test/e2e
- templates
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
| 31 | + | |
31 | 32 | | |
32 | 33 | | |
33 | 34 | | |
| |||
49 | 50 | | |
50 | 51 | | |
51 | 52 | | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
52 | 72 | | |
53 | 73 | | |
54 | 74 | | |
| |||
457 | 477 | | |
458 | 478 | | |
459 | 479 | | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
460 | 522 | | |
461 | 523 | | |
462 | 524 | | |
| |||
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
348 | 348 | | |
349 | 349 | | |
350 | 350 | | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
351 | 364 | | |
352 | 365 | | |
353 | 366 | | |
| |||
521 | 534 | | |
522 | 535 | | |
523 | 536 | | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
524 | 541 | | |
525 | 542 | | |
526 | 543 | | |
| |||
682 | 699 | | |
683 | 700 | | |
684 | 701 | | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
| 718 | + | |
| 719 | + | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
| 723 | + | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
685 | 731 | | |
686 | 732 | | |
687 | 733 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
124 | 124 | | |
125 | 125 | | |
126 | 126 | | |
127 | | - | |
| 127 | + | |
128 | 128 | | |
129 | 129 | | |
130 | 130 | | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
131 | 135 | | |
132 | 136 | | |
133 | 137 | | |
| |||
152 | 156 | | |
153 | 157 | | |
154 | 158 | | |
155 | | - | |
| 159 | + | |
156 | 160 | | |
157 | 161 | | |
158 | 162 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
5 | 47 | | |
6 | 48 | | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
7 | 57 | | |
8 | 58 | | |
9 | 59 | | |
| |||
0 commit comments