Skip to content

fix(git)!: keep the branch worker's decided writes pending, with one materializer - #413

Merged
sunib merged 35 commits into
mainfrom
fix/worker-publication-visibility
Oct 5, 2026
Merged

sunib merged 35 commits into
mainfrom
fix/worker-publication-visibility

Conversation

@sunib

@sunib sunib commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

The branch worker keeps its pending writes in order, commits them, and pushes them on one retry schedule. Through a Git outage nothing it accepted is dropped: intake pauses at a byte budget, and every GitTarget on the branch says the branch cannot publish.

Design and recovery contract: gittarget-branch-worker-pending-writes.md. Diagrams of the pipeline as built: event-pipeline-overview.md. Operator view: architecture.md.

How the write path works now

  • Pending writes, one driver (pending_writes_loop.go). Every write kind (window, atomic batch, resync, a save's empty record, a refusal's empty commit) is decided as a pending write; decide only appends. At the end of every wake, advance commits what was decided (materialize), schedules the push, and ends the outage once nothing is pending. materialize classifies each attempt in one place: committed, failed for good, or kept pending for the retry.
  • The checkout is a projection. checkoutApplied counts how many pending writes the checkout holds on top of its root; a write that failed part-way is undone locally, without a fetch.
  • One retry schedule (retry.go), 10s doubling to 5m. When due it probes for a missing parent, or materializes and pushes. Until then nothing spends a connection; a resync arriving meanwhile is answered with the failure the retry is waiting out and stays pending.
  • A missing parent holds writes back with their authors, messages and saves, and publishes them once the parent exists. Nothing is owed a snapshot.
  • A refused replay entry settles alone. When a moved remote refuses one pending write, only that write fails; the others still publish.
  • Every Git call is bounded: 2 minutes per advertisement, fetch or push session, 5 per push cycle, over HTTP and SSH. A push whose reply was lost is settled by evidence, so a save's empty commit lands once.

Through an outage

  • Intake budget (intake.go). While a retry is pending, --branch-buffer-max-size bounds everything accepted and not published: queued payload (charged at enqueue), the open window, pending writes, deferred heals and waiting saves, each item charged 1 KiB beyond its payload. A charge moves from queued to held in one locked step.
  • Pause is a latch, released only when a push lands or nothing is owed. Producers keep refused work and offer it again: the watch from its unadvanced cursor (waiting on IntakePaused instead of polling), the controller re-sends a save, a resync is gathered again.
  • Status. Every GitTarget on the branch reports Ready=False, Reconciling=True, Stalled=False (Progressing), naming since when, the cause, and whether intake is paused. New gauges: git_retained_bytes, git_retained_writes, git_intake_paused, git_oldest_retained_write_timestamp_seconds, git_next_retry_timestamp_seconds, plus git_materialization_failures_total{reason}.

Watch admission

  • A refused event is never skipped: its change filter records it only after the worker accepted it, the cursor never passes it, and a stream resumes from a stored cursor only after its own replay.
  • Each stream owns its desiredStateChangeFilter, seeded from its accepted replay, replacing a process-wide map. This fixes a replay gap that kept a stale value in Git.
  • One GitTarget watches each object through one collection; an overlapping rule is refused whole with CollectionOverlap.

Removed

recoverRetainedWrites and its six call sites, invalidateAndRefresh, the second commit lifecycle, worktreeDirty/replayRequired, the separate parent-recovery clock and its hand-off, parent recovery's scopes and the controller's snapshot-request tracker, releasedResyncs, the materializing/followUps/deciding flags, liveContentDedup, ResyncRequest.RefreshRemote and the unused EnqueueRequest.

User-visible (see docs/UPGRADING.md)

  • Breaking: a rule whose collection overlaps another of the same GitTarget is refused with CollectionOverlap (replaces ObjectSelectorConflict).
  • Breaking: gitopsreverser_git_queue_drops_total is renamed gitopsreverser_git_queue_refusals_total (same labels). It and route_failed are documented as recoverable refusals, and the suggested alert fires on a healthy branch only.
  • A save survives an unreachable remote or a missing parent; git_commit_failures_total no longer counts an unreachable remote.
  • During an outage kubectl wait --for=condition=Ready waits, where it used to return; Stalled=True/WatchError no longer fires for one.

Accepted tradeoffs

  • Pending writes live in memory: a restart loses them and their save receipts, and the watch replay restores current content only. Persistence stays with the HA plan.
  • Pending writes replay as the decisions they are. Two edits to one object onto a remote that already holds the second land as a revert and a reapply (pinned by a test).
  • A watch cursor that expires before a refused event comes back turns it into a fresh snapshot: it loses that change's own commit, and under prune.mode: OnEvent a deleted object's file stays in Git.

Validation

  • CI green at f0e953b5: lint, unit tests and all six e2e legs. full-manager needed one re-run of the known-flaky audit-route override spec on this docs-only commit.
  • Local task test-e2e 94/94 at fc02f51d; go test -race ./internal/git ./internal/watch passes; the round-trip ledger golden file is unchanged; unit coverage 79.6% → 79.9%.
  • Every review finding has a regression test, written failing first, in intake_test.go, parent_recovery_test.go, decided_write_test.go, replay_refusal_test.go and desired_state_change_filter_test.go.
  • docs/definitions.md defines "pending write" and rules out "log" and "journal" for it.

🤖 Generated with Claude Code

…aterializer

Gaps 3 to 5 of the GitTarget state of affairs are planned as one refactor: a
log of decided writes, one materializer for the checkout, and one retry
deadline. A failed rebuild then cannot drop a window, a publication failure
has one state to project, and Git deadlines wrap two call sites. The page
records the steps, the size estimate, and the settled decisions (resyncs stay
in the log; a partly failed write is cleaned locally).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Too many files!

This PR contains 130 files, which is 30 over the limit of 100.

To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch.

Upgrade to a paid plan to raise the limit.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 73e2e480-79de-44f2-8f5a-e9ee4d9afb4f
📥 Commits

Reviewing files that changed from the base of the PR and between e526889 and f0e953b.

📒 Files selected for processing (130)
  • .coverage-baseline
  • README.md
  • charts/gitops-reverser/README.md
  • charts/gitops-reverser/values.schema.json
  • charts/gitops-reverser/values.yaml
  • cmd/main.go
  • docs/INDEX.md
  • docs/TODO.md
  • docs/UPGRADING.md
  • docs/api-first-publication.md
  • docs/architecture.md
  • docs/configuration.md
  • docs/definitions.md
  • docs/design/branch-worker-event-model.md
  • docs/design/event-pipeline-overview.md
  • docs/design/gittarget-branch-worker-pending-writes.md
  • docs/design/gittarget-parent-branch.md
  • docs/design/gittarget-state-of-affairs.md
  • docs/design/label-selection-follow-ups.md
  • docs/design/metrics-observability-plan.md
  • docs/design/push-cooldown.md
  • docs/design/push-notification-and-reconcile-trigger.md
  • docs/design/source-scope-simplification.md
  • docs/future/ha-gittarget-distribution-plan.md
  • docs/interpreting-metrics.md
  • docs/spec/commitrequest-design.md
  • internal/controller/clusterwatchrule_admission_test.go
  • internal/controller/clusterwatchrule_controller.go
  • internal/controller/commitrequest_controller.go
  • internal/controller/commitrequest_finalize.go
  • internal/controller/constants.go
  • internal/controller/gittarget_controller.go
  • internal/controller/gittarget_layout_test.go
  • internal/controller/gittarget_parent_branch_test.go
  • internal/controller/gittarget_parent_recovery.go
  • internal/controller/gittarget_publication.go
  • internal/controller/gittarget_publication_test.go
  • internal/controller/gittarget_status_test.go
  • internal/controller/object_selector_rule_test.go
  • internal/controller/stream_status.go
  • internal/controller/stream_status_test.go
  • internal/controller/watchrule_controller.go
  • internal/controller/watchrule_controller_test.go
  • internal/git/ado_multiack_test.go
  • internal/git/branch_worker.go
  • internal/git/branch_worker_base_trust_test.go
  • internal/git/branch_worker_credread_test.go
  • internal/git/branch_worker_empty_repo_test.go
  • internal/git/branch_worker_fetch_metric_test.go
  • internal/git/branch_worker_metrics_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/branch_worker_test.go
  • internal/git/commit_executor.go
  • internal/git/commit_request_attach.go
  • internal/git/commit_request_attach_loop.go
  • internal/git/commit_request_attach_test.go
  • internal/git/commit_request_empty_test.go
  • internal/git/commit_request_withdraw_test.go
  • internal/git/commit_window_metrics_test.go
  • internal/git/commit_window_timers_test.go
  • internal/git/decided_write_test.go
  • internal/git/deleted_remote_branch_test.go
  • internal/git/dirty_worktree_recovery_test.go
  • internal/git/git.go
  • internal/git/git_atomic_push.go
  • internal/git/git_operations_test.go
  • internal/git/git_push_advertisement_test.go
  • internal/git/git_roundtrip_ledger_test.go
  • internal/git/git_smart_fetch.go
  • internal/git/gittargetignore_writer_test.go
  • internal/git/helpers_test.go
  • internal/git/intake.go
  • internal/git/intake_test.go
  • internal/git/network_bound.go
  • internal/git/network_bound_test.go
  • internal/git/open_window.go
  • internal/git/parent_recovery.go
  • internal/git/parent_recovery_test.go
  • internal/git/pending_writes_loop.go
  • internal/git/provider_repoint_test.go
  • internal/git/publication.go
  • internal/git/publication_retry.go
  • internal/git/publication_test.go
  • internal/git/refresh.go
  • internal/git/refresh_test.go
  • internal/git/refusal_observation_test.go
  • internal/git/refusal_touch.go
  • internal/git/refusal_touch_test.go
  • internal/git/replay_failure_test.go
  • internal/git/replay_refusal_test.go
  • internal/git/resync_flush.go
  • internal/git/resync_flush_test.go
  • internal/git/resync_heal_test.go
  • internal/git/resync_push_test.go
  • internal/git/retry.go
  • internal/git/retry_test.go
  • internal/git/secret_write_test.go
  • internal/git/types.go
  • internal/git/worker_manager.go
  • internal/git/write_gate.go
  • internal/git/write_gate_test.go
  • internal/reconcile/git_target_event_stream.go
  • internal/telemetry/exporter.go
  • internal/telemetry/gauges.go
  • internal/types/collection.go
  • internal/watch/collection_overlap.go
  • internal/watch/collection_overlap_test.go
  • internal/watch/desired_state_change_filter.go
  • internal/watch/desired_state_change_filter_test.go
  • internal/watch/event_router.go
  • internal/watch/event_router_test.go
  • internal/watch/helpers_test.go
  • internal/watch/live_content_dedup_test.go
  • internal/watch/manager.go
  • internal/watch/object_selector_test.go
  • internal/watch/owner_test.go
  • internal/watch/prune_declaration_test.go
  • internal/watch/refused_admission_test.go
  • internal/watch/source_namespace_planning_test.go
  • internal/watch/stream_readiness.go
  • internal/watch/target_watch.go
  • internal/watch/target_watch_test.go
  • internal/watch/terminating_snapshot_test.go
  • internal/watch/watched_type_resolver.go
  • internal/watch/watched_type_table.go
  • test/e2e/object_selector_e2e_test.go
  • test/e2e/source_namespace_e2e_test.go

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Review on demand using usage pricing
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

sunib and others added 4 commits October 2, 2026 13:04
The checkout is the projection of the retained writes: the remote tip they
were planned on plus one commit each. Three flags guessed whether it still
was (worktreeDirty, replayRequired, and the hasPendingCommits parameter), six
recoverRetainedWrites call sites consulted them, and four functions rebuilt
the projection in slightly different ways.

checkoutApplied now counts the retained writes the checkout holds. Unknown is
a dirty worktree, which only a reset clears and a push never does; a reset
sets it to zero, so with writes retained it says their commits must be
replayed. materialize is the one place the loop resets and replays, and the
loop's commit refuses a checkout that is not current, so a path that skips
materialize fails instead of committing leftovers.

Removed: recoverRetainedWrites, invalidateAndRefresh, refreshRemoteForResync,
the duplicate replay tail, replayRequiredState, worktreeDirtyState, and the
hasPendingCommits parameter. The resync prelude collapses to one materialize:
its RefreshRemote branch and its default branch already did the same thing.

No behavior change: the round-trip ledger is byte-identical and every test is
green. A reset made outside the loop now also marks retained writes for
replay instead of leaving them naming discarded commits.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… already does

A forced GitTarget recheck asked the worker to fetch the remote tip before its
snapshot was judged. Every resync already fetches before it is judged, and the
worker's two branches for it did the same thing, so the flag changed nothing
from the watch stream through the coalescing to the worker. A forced recheck
still restarts its streams; only the hand-off to the worker is gone.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A write that failed after staging part of its change left the worktree dirty,
and the next commit fetched and reset from the remote to clear it. The
checkout is the projection of the retained writes, so the commit a batch
started on is all the cleanup needs: a failed batch is now reset to it
locally, its leftovers discarded, and base trust kept. Only a failed write
that cannot be undone locally still leaves the worktree dirty for a reset
from the remote, so a `recovery` fetch now points at a reset that went wrong,
a parent change, or a replay that did not finish.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
When retained writes needed a rebuild and its fetch failed, finalizing a
window dropped it and failed the CommitRequest riding it; only a missing
parent asked for a snapshot to re-derive the lost writes, and every new
window spent a fetch retrying the rebuild.

A closed window is now a decided write: it enters the log before it is
committed, and materialize commits the decided writes not committed yet,
after rebuilding the ones that were. A failure of the write itself (a refused
plan, a write that cannot be made) is still terminal for that write alone; an
unreachable remote leaves every decided write in the log for the publication
retry, and the request riding one stays WaitingForPush. While the retry is
pending, a decision that would need a connection to commit waits for it. The
CommitEmpty record of a request that reached no window is decided the same
way. A missing parent still drops the window to a snapshot until the log is
bounded.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

sunib and others added 16 commits October 2, 2026 13:46
Review of the decided-window change found two defects and a structural gap.
Windows and saves took decide -> materialize, while atomic batches, resyncs
and refusal touches still took materialize -> commit -> retain; the executor
re-entered itself through refusal handling; and pcr.committed came to mean
"decided".

Every write kind is now decided into the log and committed by materialize, in
branch_log.go. One place classifies an attempt: committed, failed for good, or
left for the retry because the remote could not be reached. A write's origin
(the resync caller, the atomic request, the refusal a touch answers) rides
with it, so its outcome is settled there. materialize is never re-entered: a
refusal's empty commit decided while a refused write is settled is appended
for the running pass to commit in order, and no push starts inside a pass.
l.commit, retain, the commit guard, pcr.committed and rebuildPendingWrites are
gone.

Fixes from review:
- A write committed later than its decision re-reads its prune policy, as a
  replay does. A delete decided during an outage deleted a file the operator
  had since set to prune: Never.
- Parent recovery no longer opens an obligation with nothing owed. A save
  whose empty record failed for a missing parent left the target
  RecoveringParentBranch with nothing to publish.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ill owes

A failed publication and a missing parent each kept their own deadline, and
the two had to hand work back and forth: a failure while parent recovery was
open deferred to recovery's deadline, or every new commit retried at once.

One schedule, retry.go, replaces publicationRetry and parent recovery's
backoff, next-probe time and timer. What is due when it fires decides the
attempt: while parent recovery is open, one advertisement looking for the
parent; otherwise, materializing the log and pushing it. Whoever observes a
failed attempt schedules the next one, once, and a new parent latch starts
the schedule over so the first probe is one initial backoff away, as before.
The push timer is only the success cooldown again.

publication_retry.go, deferToRecovery and the hand-off between the two clocks
are gone. Retention is unchanged; the round-trip ledger is unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ssion

A window, save or resync decided while the parent branch was missing was
dropped; parent recovery remembered its (GitTarget, collection) scope, and
once the parent returned the controller forced a recheck whose snapshot
re-derived the cluster's state. That lost the live author and message of
every dropped window, failed every save riding one, and needed per-scope
bookkeeping to know when a snapshot had been published.

Writes decided while the parent is missing now stay in the log like any other
a remote failure holds back, and are published when the probe finds the
parent. A resync the remote holds back answers its caller with the error at
once and stays in its place, so the writes decided around it keep their order;
resyncs are never replaced. Nothing is dropped, so nothing is owed a snapshot:
parent recovery's scopes, awaitingPush, the snapshot-request sequence and the
controller's snapshot-request tracker are gone.

The log is bounded at admission instead. While its retained writes hold the
branch's byte budget and a failed attempt waits for its retry, the worker
refuses new writes, saves and resyncs through the existing queue-full
contract: the watch records its cursor only after a write is accepted, so the
reconnect delivers a refused event again. A healthy branch never closes.

A pending retry now defers a new decision only until it is due.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A refusal a resync discovers decides its empty commit while the resync's
outcome is settled, inside the commit pass. Nothing pushes from inside a pass,
and the resync's own caller schedules a push only when the resync committed,
so the empty commit was made and never left the checkout. The refusal e2e
caught it; the unit fixture pushed by hand and hid it.

A write decided during a pass is now a follow-up: whoever started the pass
schedules its push once the pass ends, and a pass started by the push itself
publishes it with the rest.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A replay used to abort at its first error, so one retained write that the
moved remote now refused blocked every write behind it indefinitely.

replayPendingWrites undoes a refused write on its own, goes on with the
rest, and stamps the entry with its refusal; checkoutApplied counts only
the writes the checkout holds. After either replay (the loop's rebuild,
the push cycle's after a rejection) the loop takes stamped entries out of
the log and settles each as a refusal at first commit is settled: the
refusal is reported and a save riding it fails. Any other failure still
abandons the replay and keeps everything for the retry.

A committed resync is now marked answered, so a replay that refuses it
later reports the refusal on the target instead of answering its caller
a second time.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…Git is down

Record the recovery contract for a saturated branch worker (pause new
payloads, keep accepted work and the retry schedule, resume after the
backlog settles), the three meanings of replay, and the source review's
findings at 373bf8d that gate the next steps (dedup before admission,
incomplete byte budget, resync fetches during backoff, polling watch
recovery). Steps 5a/5b/5c split out; Redis stays deferred.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…er accepted it

A refused UPDATE was lost on cursor resume: the content hash was recorded
before routing, so the redelivered frame matched the hash of an event the
worker never took, was skipped as unchanged, and advanced the cursor past
it. The dedup check now reads the cache before routing and records the
baseline only after Enqueue returned true, swapping from the entry the
check saw so an overlapping stream's acceptance is never overwritten by a
stale baseline (a conflict clears the entry: no baseline is safe, a wrong
one is not).

Two more holes on the same boundary:
- the shutdown arm returned the never-enqueued event's resourceVersion, so
  the cursor moved past it;
- runTargetWatch resumed from a cursor after the first session ended however
  it ended, so a stream whose snapshot the worker refused (admission
  backpressure) resumed from a previous stream's cursor and skipped its own
  replay, sweep and render-fidelity report. It now resumes only after its
  own replay recorded a cursor.

EnqueueRequest is removed: it hid its enqueue boolean and had no caller.

Step 5a of docs/design/gittarget-branch-worker-log.md, with the producer
inventory and the limits left for 5b.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A picture-first overview of the path from a watch frame to a pushed
commit: watch stream replay/resume rules, the three meanings of
"replay", resync coalescing and fences, the branch worker's inputs and
window closing, the log's lifecycle, publication with a rejection,
retry and admission, a save end to end, what is an event today, and an
honest list of what is still missing and who owns it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ply by evidence

Measured first against go-git v6.0.0-alpha.5 with servers that accept a
connection and then stall: over HTTP, go-git's context-taking API returns
at the deadline, but SmartFetchFrom, CheckRepo and the advertisement used
the context-free List and Fetch and hung; over SSH, go-git uses the
context only to dial, so the SSH handshake and every git-protocol read
hung past an expired context, and worker shutdown waited with them.

- listRemoteRefs, the fetch in SmartFetchFrom, CheckRepo and
  advertiseRemoteBranch use ListContext/FetchContext and take a context.
- Each call adds a dialer whose connection closes when the call's
  context ends, which is what unblocks SSH. Nothing runs the operation
  on another goroutine, so nothing outlives it with the checkout.
- gitCallTimeout (2m) bounds each advertisement, fetch or push session,
  inside the library functions, so the GitProvider controller's check is
  bounded too; gitPublishTimeout (5m) is one deadline over the whole push
  cycle, contention retries and replays included.
- A push that fails without a rejection already probed the remote; when
  the probe finds the branch at our local head, the push landed and only
  its reply was lost, so the cycle settles as published instead of
  replaying (which landed a save's empty commit twice). Only the exact
  head counts; a remote that moved on past our commits still replays.

runPushCycle's failure handling moves to afterFailedPush and its success
bookkeeping to notePushSucceeded. Step 6 of
docs/design/gittarget-branch-worker-log.md, with the measurement table.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…get, not just Ready

The #397 fix retried the direct reconcile until the rule's Ready was
False/Progressing, but with no streams in envtest Ready is
False/Progressing either way, so the wait could pass on its first try
while the reconciler's cached read of the GitTarget still had no
conditions, leaving GitTargetReady=Unknown. It recurred in CI on #413.
The wait now also requires GitTargetReady False/Progressing, the
condition the test asserts.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Step 5a2 keeps the filter that drops Git-useless live UPDATEs (it guards
commit windows against unattributed /status updates and keeps status
churn out of the FIFO) but moves its memory from one process-wide map
onto each stream. That removes the cross-stream compare-and-swap from
5a, frees entries with the stream, and closes a loss the shared memory
has today: a replay inside a stream never updates it, so an object that
returns to its pre-gap content after the replay is dropped as unchanged.
Naming and the reset-versus-seed choice are left as decisions for
review. The overview shows today's shared memory and the planned shape.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A GitTarget now watches each object through one collection. Two rules
whose collections overlap structurally (same type, and the same
namespace or one of them all namespaces) are refused whatever their
object selectors; before, only a selector disagreement was. An exact
duplicate is one collection and still shares one stream; disjoint
namespaces and separate GitTargets stay independent.

Oldest-rule precedence and whole-rule refusal are kept. The reason is
now CollectionOverlap (was ObjectSelectorConflict), and the message
names both rules and both scopes. This is the prerequisite for giving
each stream its own unchanged filter: with one stream per object, no
second producer can deliver a stale copy into another author's window.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The unchanged filter was one process-wide map shared by every stream,
coordinated by compare-and-swap between overlapping streams, and a
replay never updated it. Two defects followed: after a replay found Z,
a live return to the pre-replay content X matched the stale baseline
and was dropped, leaving Git at Z; and a replayed object had no
baseline, so its first status-only update reached the worker
unattributed and could split another author's commit window.

desiredStateChangeFilter is a UID-to-hash map owned by one stream,
kept across its reconnects and released with it. A live event's hash is
recorded only once the worker accepted it, and an accepted DELETE
clears the UID. Each replay, initial-events or LIST fallback, gathers
hashes with the same sanitizer and replaces the map only once the worker
accepted the snapshot; a refused or unfinished replay installs nothing.
With overlapping collections refused per GitTarget, the stream is the
only producer for an object, so Manager.liveContentDedup and its
compare-and-swap are gone. The unchanged outcome and the step 5a
accepted-event and cursor guarantees are kept.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Cancelling a stream returns before its goroutine finishes its last session, which still records
metrics. A test that returned on cancel left that goroutine reading the global exporter while the
next test reset it, so `go test -race ./internal/watch` failed: once through the new
replacement-stream overlap test, and once through an older plan test.

The manager now counts the goroutines running a target watch, and the tests that start real
streams wait for them in cleanup. The package passes under -race.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…included

Admission closes when the log holds the retained-byte budget during an outage, but the log was
charged only each write's payload. A save's empty record and a refusal's empty commit carry none,
so a branch whose remote was down kept admitting saves against a budget it never reached: ten
empty saves were retained against a one-byte budget at zero bytes.

Every decided write is now charged its payload, its message, and a fixed overhead for its
bookkeeping. The charge is fixed at the decision and refunded exactly when the write leaves the log.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
decide let a resync materialize while a failed attempt waited for its retry, because its caller is
waiting to hear what it found. Each resync then spent a fetch before the retry deadline, and with
slow failures each one held the shared worker for the length of that call. A write decided behind
a retained resync reached the same fetch through the pass.

The retry schedule now records the failure it is waiting out. A resync during backoff is answered
at once with that failure, as one the remote refused is, and stays in the log; it is applied when
the retry is due. A write behind an uncommitted resync is no longer treated as local.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sunib sunib changed the title fix(git): keep the branch worker's decided writes in a log with one materializer fix(git)!: keep the branch worker's decided writes in a log with one materializer Oct 4, 2026
sunib and others added 6 commits October 4, 2026 18:31
A resync coalesced into a queued marker is told it was enqueued, and the marker's own request is
answered as superseded. When a write then queued behind the marker made a newer resync take its own
slot, the key was released and the marker ran the request it first carried: the one already
answered. The coalesced request was never run and never answered, so its caller waited out its
timeout for a reply that could not come. Git still converged through the newer resync.

A released marker now runs the request it held when it was released.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
During an outage the budget was read once per loop iteration against what the loop already held,
so producers could fill the whole FIFO between two iterations however far past the budget that
went. It also counted neither queued payload, deferred heals, nor saves waiting for a window, and
it reopened as soon as the log dipped below the budget, even while the remote still refused pushes.

An intake gate now charges every write, save and resync at enqueue: its payload, sized once by the
producer off the loop, plus a fixed overhead per item, so the budget bounds the item count too. The
loop publishes what it holds: the window, the log, deferred heals and waiting saves. While a retry
is pending, a payload that would cross the budget is refused and pauses the branch. The pause is a
latch, released only when the retry clears; room under the budget again or a remote that can be read
but refuses the push keep it. A snapshot larger than the whole budget is refused during an outage
with a message naming both sizes, and accepted once the branch publishes again. Withdrawals,
refresh ticks and shutdown are lifecycle work and never pass the gate.

IntakePaused returns a channel closed when intake reopens, for producers to wait on.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A target watch whose session ended reconnected after its two-second backoff, whatever the reason.
On a branch that had paused intake that meant gathering a snapshot, or delivering an event, that the
branch refused again, every two seconds for as long as the outage lasted.

A stream now waits on the branch's IntakePaused channel before reconnecting, looks again after
every wake-up because the branch can pause again first, and falls back to a one-minute re-check.
Its cursor stays where the last accepted event left it.

Marks step 5b built in the branch-worker log plan, and updates UPGRADING, interpreting-metrics and
the CommitRequest spec for the intake budget.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The overview now shows the intake gate and its pause latch, the watch stream waiting for a paused
branch instead of reconnecting every two seconds, the retry deadline answering a resync with the
failure it waits out, and the coalescing fix. The gap list keeps only what 5b left open: the budget
counts serialized bytes, a healthy branch's FIFO is bounded by count, and loop-derived work can
overshoot by a bounded amount.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nd held saves

Through a Git outage a GitTarget reported Ready=True, and once intake paused it reported
Stalled=True with WatchError, because its refused streams were graded as broken watches. A save
waiting for its push said only that it was waiting.

The branch worker now publishes one report: whether a failed attempt waits for its retry, since
when (stable across retries), the last cause, and whether intake is paused. It is replaced, and the
branch's GitTargets enqueued, only when one of those changes, so a long outage writes no status
per retry. Every GitTarget on the branch reports Ready=False, Reconciling=True, Stalled=False under
Progressing, with the start, the cause and the pause in the message; terminal gates still win, and
a missing parent is left to ParentBranchNotFound. A stream that waits for its branch is graded
Replaying/BranchIntakePaused. A save in WaitingForPush carries the same message.

New metrics: git_retained_bytes, git_retained_writes, git_intake_paused,
git_oldest_retained_write_timestamp_seconds and git_next_retry_timestamp_seconds, read at scrape
time, and git_materialization_failures_total{reason} for the failure before any push cycle.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The branch-worker log plan records what step 5c built and points the next-step prompt at step 7.
The pipeline overview describes the publication report and drops the status gap it closed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…rough an outage

Step 7 of the plan. Architecture gains a section on a remote that cannot be reached: decided writes
are kept and retried on one schedule, every Git call is bounded, the outage budget pauses intake
until a push lands, and every GitTarget on the branch reports it. The event model, the state of
affairs page, the HA plan's prerequisite, the CommitRequest spec, and INDEX now say what #413 built
and what is left: recording the other FIFO inputs as transitions, then persistence.

The plan is marked built and stays in docs/design, because its recovery contract is still the
reference other pages link to. write_gate.go's effective-point comment already matched what was
built and is unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sunib
sunib marked this pull request as ready for review October 4, 2026 19:53
sunib and others added 4 commits October 4, 2026 20:43
Review of fc02f51 reproduced four defects, and asked for one structural step.

decide now only appends. One driver, advance, runs at the end of every wake:
it materializes what was decided, schedules the push, and closes the retry and
parent recovery once the log is empty. Handlers no longer commit or push, so
the materializing, followUps and deciding flags and decision numbering go.

- Recovery ends when nothing is owed. A held resync that found nothing to
  change once the parent returned emptied the log without a push, and left
  recovery, its retry and the intake pause open for good.
- A handled item's charge moves from queued to held in one locked step.
  It was released before the loop published what it held, so a producer in
  between found room the branch did not have.
- The FIFO holds a resyncMarker with the current request and its charge.
  A marker used to be the first request of its key, so the queue kept every
  snapshot coalescing replaced; releasedResyncs is gone.
- A window charges each event it keeps, deletes included, and the save
  attached to it. A decided write is never charged more than the window or
  save it came from.

Every commit now re-reads its write's prune policy, from the informer cache.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ng the last

Each decided write is judged against the tree the one before it left, so two
edits to one object replayed onto a remote that already holds the second one's
content land as a revert and a reapply. The final tree is right, and each
decision keeps its commit and the save riding it. This is accepted rather than
collapsed, which would cost a save on the first edit its commit; the design doc
says so next to the replay contract.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… with it

removeAt shortened the log in place without clearing the vacated slot of its
backing array, so a resync that committed nothing kept its whole snapshot
reachable while the budget said the branch held nothing. slices.Delete clears
it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
"Log" crept into the domain on this branch for something that was never new:
the ordered pendingWrites the worker already had. It also names diagnostic
logs and suggests a durable journal, which this is not.

- branch_log.go is pending_writes_loop.go, beside pending_writes.go, as
  commit_request_attach_loop.go sits beside commit_request_attach.go.
- gittarget-branch-worker-log.md is gittarget-branch-worker-pending-writes.md,
  and every link follows it.
- Comments, log messages, test names and docs say pending writes; decide,
  materialize and publish keep their names.
- definitions.md defines "pending write" and rules out "log" and "journal"
  for it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sunib sunib changed the title fix(git)!: keep the branch worker's decided writes in a log with one materializer fix(git)!: keep the branch worker's decided writes pending, with one materializer Oct 4, 2026
sunib and others added 3 commits October 4, 2026 21:18
… what they do

A refused enqueue is not lost work any more: the watch keeps its cursor and
delivers the event again, the controller re-sends a save, a resync is gathered
again, and the next reconcile asks for a refresh. It costs a write's own commit
only when its watch cursor expires first. The metric guide, its alert, the
outcome classes, the chart's queue-depth help and the comments still called it
loss; they now call it a refusal, and route_failed and git_queue_drops_total
move to the recoverable class. The alert fires on a healthy branch only, since
an outage's refusals are the intake pause, which has its own.

- PendingWrite.materialized is committedOnce: it stays set after a reset
  discards the commit, so it never meant the checkout holds it.
- settleUnreachable is replyToDeferredResyncs: it settles nothing, it answers
  the callers of resyncs that stay pending.
- The HA plan and push-cooldown no longer list built work as outstanding, and
  a telemetry comment no longer says writes stay in the log.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A refused enqueue is not lost: its producer offers the work again, and only a
watch cursor that expires first costs a write its own commit. The counter also
counts refusals while a branch has paused intake, not only a full queue. Its
name said otherwise, so it now says refusals. The labels are unchanged.

BREAKING CHANGE: gitopsreverser_git_queue_drops_total is no longer emitted;
dashboards and alerts must use gitopsreverser_git_queue_refusals_total.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…prune policy

A refused write whose watch cursor expires is recovered by a fresh snapshot,
which restores current content but cannot infer a delete from absence under
the default prune.mode OnEvent: the deleted object's file stays in Git. The
metric guide, the counter comments, the queue-depth help and the upgrade note
said only the write's own commit was lost; they now say both, and the guide
links the recovery contract.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sunib
sunib merged commit b6492d8 into main Oct 5, 2026
37 of 38 checks passed
@sunib
sunib deleted the fix/worker-publication-visibility branch October 5, 2026 12:16
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