[sunib]

Stage 1 of docs/design/concurrent-author-windows.md: commit-window management moves behind one type, a clock that tests can step drives the window timers, and a reference model of today's single-window rules checks the worker. There is no behavior change. Every existing test passes, and no existing test file was edited.

Stacked on #462 (docs only; it holds the design doc).

What changes

A clock for the window timers (BranchWorker.now(), already used by the retry schedule). It now also drives the idle and max-duration deadlines, the opening time a window's duration is measured from, and a CommitRequest's attach deadline. The loop records the worker-clock time each of its commit and attach timers is armed for (commitTimerAt, attachTimerAt). A test steps the clock to that time and delivers that timer's wake by hand, as run does: no sleeping, and no rewinding a deadline. A deadline the worker armed no timer for is never delivered. The timers themselves stay real. Unrelated time.Now calls (push cooldown, apply windows, outcome retention) are unchanged.

commitWindows (internal/git/window_set.go) holds the open windows in opening order, the sequence number each window takes when it opens, and the charge each window carries. Every caller now says which window it means, or all of them:

Path	Window
a write (appendLiveEvent)	the one it joins (forWrite); the ones it must close first (blocking, identity_change)
a save (handleAttachCommitRequest, attach, saveInProgress)	the one it may attach to (forRequest); a foreign one (foreignTo); the one carrying it (carrying)
timers (closeOrArmWindow)	each window whose deadline passed; the commit timer is armed for the earliest deadline left
push hint (foreign_commit)	every window without an attached request
resync, apply flush, byte cap, shutdown	all, oldest first (finalizeOpenWindowWithReason)
finalize and drop	one (finalizeWindow, dropWindow)

The set allows exactly one window, so close reasons, close order, retained-byte charges and request settlement are unchanged. Closing a window releases exactly that window's charge, not the whole charge as before; with one window the two are equal.

Why the set is embedded in the loop: about 150 lines across 26 test files read or assign loop.openWindow and loop.windowBytes directly. Embedding keeps both names as the set's fields, so those tests compile and pass unchanged. Stage 2 replaces the single slot with an ordered list. Those tests then switch to accessors, and that change is where their meaning is reviewed.

The reference model

internal/git/window_model_test.go is a small, independent implementation of today's rules. It shares only the event fixtures, the per-event charge and the close-reason names with the worker. It runs on the echo fixture: a real Git server, a GitTarget whose applier claim has a recording requester, and outside clones. The model and the worker get the same random inputs.

Checked after every step:

• the open window: author, object count, attached save, deadline and its reason, charge;
• the timers armed: the commit timer for the window's close, the attach timer for the earliest waiting save;
• the registered saves, attached or waiting, and their deadlines;
• the retained-byte charge;
• the close reasons in order, read from the finalize log;
• every save's outcome, settled exactly once, counted from the resolve and refuse logs;
• every published commit: author, message, every path it changes (the bootstrap files only in the folder's first commit), and each document whole, parsed as YAML 1.2;
• per document, landed writes in acceptance order, so an older accepted write never lands after a newer one.

Checked drained (after shutdown, or after the clock runs past every deadline): Git's tree is the model's, path for path and document for document, the retained charge is zero, and saves given back at shutdown have no outcome.

Inputs: writes by three authors (new values, undos, no-ops, deletes and recreates); saves (attach_next, attach timeouts of 0s, 5s and 20s, maxDuration of 0s, 15s and 40s, with and without idleTimeout); outside pushes with a push hint; apply flushes (a due Repair, answered by the test applier); resyncs; buffer pressure (a cap of 1 byte for one write); clock steps, which fire each armed timer when it comes due through that timer's own wake, in either order when two are due at once; and shutdown.

Building the model corrected three wrong expectations of mine. Each is now part of the model, and none is a bug:

• a save is never trimmed, so its replay onto an unseen outside push re-asserts every object it captured (C2);
• an apply goes for the tip the worker knows, so an outside push it has not found yet is not in it;
• an exited loop keeps counting saves it gave back in heldBytes; it retains nothing for Git.

Sensitivity. I broke the worker eight ways in a scratch worktree and ran 30 seeds against each. The model caught seven: an attach charge left out, idle winning a tie with max-duration, a push hint closing a save's window, no WindowMismatch, a close keeping its charge, no SaveInProgress for a waiting save, and a flush that ignores a registered save. It did not catch reversing first-come-first-served among waiting saves. That rule cannot be observed today: one author on one GitTarget never has two saves waiting at once, because the second is refused SaveInProgress.

After review, two more: dropping the commit timer's re-arm in closeOrArmWindow, and arming the attach timer a second late. The model and the clock tests now catch both. A commit carrying the expected change plus a stray file, or an extra data field, is rejected too.

Runs: 6 seeds × 40 steps by default (about 12s), 2 seeds under -short. WINDOW_MODEL_SEEDS, WINDOW_MODEL_STEPS and WINDOW_MODEL_SEED widen a run or replay one seed. A wider run of 200 seeds × 60 steps (about 8 minutes) passed with no divergence.

A real bug, not fixed here

TestApplyWindow_AWithheldApplyIsNotCommittedUnderTheNextEdit (internal/git/apply_window_carry_test.go) is skipped. GITOPS_REVERSER_KNOWN_BUGS=1 runs it, and both cases fail. It is recorded in docs/TODO.md.

S1 withholds an apply that lands over an edit decided after the flush mark (E1, a: a2), and a Repair is queued to put E1 back. But the apply did land on the cluster. The next edit of that object by the same author changes only b, yet it carries the apply's a: a1, and the whole-object window commits it under that author's name:

• with E1 still in the window, E1 is replaced and never reaches Git;
• with E1 already published, the author's commit reverts their own E1.

Either way, the Repair then applies the loss to the cluster. I found this from the brief's candidate with a hand-written test; the model cannot produce applier-manager writes yet (below).

Inputs the model cannot express yet

• Several GitTargets on one branch (identity_change by target), and unattributed or unresolved writes.
• Objects that share a file, kustomize layouts, placement templates, generator sources: the footprint cases of stage 2.
• Failed publications, the retry, and pending writes held across wakes by the push cooldown: the driver pushes at once, and every push succeeds or replays.
• Applier-manager writes: late applies, echoes, apply-window edits, and Repair (the bug above).
• CREATE events (adoption makes an apply due), resync sweeps under prune.mode: Always, and resyncs while Git is ahead (F3 discards the snapshot): the driver resyncs only when Git is not ahead.
• GitTarget spec changes mid-window (timers, suspend, the render-fidelity gate).
• whenNothingToCommit: CommitEmpty record commits, catchUp: AfterPush, and withdraws.
• Shutdown with a failing final push.
• Byte pressure from pending writes rather than one window.

For stage 2

• finalizeWindow still stops the commit timer before it closes a window. With one window that is correct. With several, a close that leaves other windows open (push hint, attach_next, identity_change) must re-arm the timer for them, as closeOrArmWindow already does. The model's timer check will fail on that until it does.
• Before a second window is allowed, the tests that read loop.openWindow and loop.windowBytes move to accessors.
• forRequest returns the first matching window. Stage 2 keys windows by (GitTarget, author, attribution class), and matching by NamesActor may let two windows match one request.

Validation

task fmt, generate, manifests, vet, lint, test: pass. task test-e2e passed locally (112 of 175 specs ran, 0 failed) before the review follow-up, which touches only tests and two recorded timer times.

🤖 Generated with Claude Code

[coderabbitai]

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 59 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

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 84774d65-e945-4292-a6a7-0a2d22c2acd2

Commits

Reviewing files that changed from the base of the PR and between 4435d90 and 946e39b.

Files selected for processing: 19

• docs/TODO.md
• docs/design/branch-worker-event-model.md
• docs/design/concurrent-author-windows.md
• docs/design/window-field-ownership-and-apply-hold.md
• internal/git/applier_claim.go
• internal/git/applier_claim_test.go
• internal/git/apply_window_carry_test.go
• internal/git/branch_worker.go
• internal/git/commit_request_attach_loop.go
• internal/git/commit_window_clock_test.go
• internal/git/intake.go
• internal/git/open_window.go
• internal/git/parent_recovery.go
• internal/git/refresh.go
• internal/git/resync_flush.go
• internal/git/window_model_test.go
• internal/git/window_set.go
• internal/git/window_set_test.go
• internal/git/write_gate.go

• Autofix · 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!