Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions api/v1alpha3/commitrequest_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
// +kubebuilder:validation:XValidation:rule="self.gitTargetRef == oldSelf.gitTargetRef && has(self.message) == has(oldSelf.message) && (!has(self.message) || self.message == oldSelf.message) && has(self.window) == has(oldSelf.window) && has(self.whenNothingToCommit) == has(oldSelf.whenNothingToCommit) && (!has(self.whenNothingToCommit) || self.whenNothingToCommit == oldSelf.whenNothingToCommit)",message="CommitRequest spec is immutable after creation"
// +kubebuilder:validation:XValidation:rule="!has(self.whenNothingToCommit) || self.whenNothingToCommit != 'CommitEmpty' || has(self.message)",message="whenNothingToCommit: CommitEmpty requires spec.message, because a message is all an empty commit records"
type CommitRequestSpec struct {
// GitTargetRef names the GitTarget whose open commit window to finalize.
// GitTargetRef names the GitTarget whose commit window this request attaches to.
// The GitTarget must be in the same namespace as this CommitRequest.
// +required
// +kubebuilder:validation:XValidation:rule="self.name != ''",message="spec.gitTargetRef.name must not be empty"
Expand Down Expand Up @@ -192,9 +192,10 @@ type CommitRequestStatus struct {
// +kubebuilder:printcolumn:name="Message",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].message`,priority=1
// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`

// CommitRequest is a one-shot "save" signal: creating one finalizes the open
// commit window for the referenced GitTarget instead of waiting for the
// silence timer. The resulting commit hash is reported back in status.
// CommitRequest is a one-shot "save" signal. It attaches to its author's current or next
// commit window on the referenced GitTarget, closes that window on its own timers instead of the
// target's, and gives the resulting commit its message. The commit hash is reported in status
// once the commit is on the remote.
type CommitRequest struct {
metav1.TypeMeta `json:",inline"`

Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha3/gittarget_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ type GitTargetSpec struct {
// the identity talking to the remote and stay on GitProvider. Migration: docs/UPGRADING.md.

// Commit configures how this target's writes are batched into commits, and how those commits
// are phrased. Omitted, writes coalesce over a 5s rolling silence window and use the built-in
// message templates.
// are phrased. Omitted, writes coalesce over a 5s rolling silence window, a window closes at
// most 1m after it opened, and commits use the built-in message templates.
// +optional
Commit *GitTargetCommitSpec `json:"commit,omitempty"`

Expand Down
9 changes: 5 additions & 4 deletions config/crd/bases/configbutler.ai_commitrequests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ spec:
schema:
openAPIV3Schema:
description: |-
CommitRequest is a one-shot "save" signal: creating one finalizes the open
commit window for the referenced GitTarget instead of waiting for the
silence timer. The resulting commit hash is reported back in status.
CommitRequest is a one-shot "save" signal. It attaches to its author's current or next
commit window on the referenced GitTarget, closes that window on its own timers instead of the
target's, and gives the resulting commit its message. The commit hash is reported in status
once the commit is on the remote.
properties:
apiVersion:
description: |-
Expand All @@ -76,7 +77,7 @@ spec:
properties:
gitTargetRef:
description: |-
GitTargetRef names the GitTarget whose open commit window to finalize.
GitTargetRef names the GitTarget whose commit window this request attaches to.
The GitTarget must be in the same namespace as this CommitRequest.
properties:
name:
Expand Down
4 changes: 2 additions & 2 deletions config/crd/bases/configbutler.ai_gittargets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ spec:
commit:
description: |-
Commit configures how this target's writes are batched into commits, and how those commits
are phrased. Omitted, writes coalesce over a 5s rolling silence window and use the built-in
message templates.
are phrased. Omitted, writes coalesce over a 5s rolling silence window, a window closes at
most 1m after it opened, and commits use the built-in message templates.
properties:
message:
description: |-
Expand Down
14 changes: 9 additions & 5 deletions docs/INDEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,13 @@ decided), **design, decided** (decision made, not built), **partly built**, **bu
**deferred** (parked, kept as a decision record). The label is the first thing in the page,
so you never have to read a proposal to find out it already shipped.

### Open (27 pages)
### Open (29 pages)

| Doc | Open question |
|---|---|
| [`gittarget-parent-branch.md`](design/gittarget-parent-branch.md) | **built in #407, not merged yet.** `spec.parentBranch`: a target stays on standby until a write needs a branch, then creates its write branch from the parent's tip as checked at publication. The contract as built, the defect it started from, prior art and naming. Open: parent per `GitProvider.status.branches` entry. |
| [`branch-worker-event-model.md`](design/branch-worker-event-model.md) | **proposal; its first fix is built in #412.** The quiet-branch `WaitingForPush` failure (a failed push now retries on its own backoff) and the Git operation deadlines still to do. Worker ownership, FIFO inputs, save and deadline semantics, and the transition boundary for later durability. Journal storage and rollout belong to the HA plan. |
| [`gittarget-state-of-affairs.md`](design/gittarget-state-of-affairs.md) | **snapshot, 2026-10-02.** What is built for `GitTarget` and the branch worker, which page owns which decision, the known gaps ranked by user impact (verified in code), and the recommended order. Read it first to find the right page on this topic. |
| [`gittarget-parent-branch.md`](design/gittarget-parent-branch.md) | **merged in #407.** `spec.parentBranch`: a target stays on standby until a write needs a branch, then creates its write branch from the parent's tip as checked at publication. The contract as built, the defect it started from, prior art and naming. Open: parent per `GitProvider.status.branches` entry. |
| [`gittarget-parent-hardening.md`](design/gittarget-parent-hardening.md) | **done in #407.** Parent changes on a live worker, an empty repository gaining a ref, an unresolved default branch, recovery that cannot strand work, and the refused upload, without adding a pull to the fast path; plus the two recovery fixes from the review at `93120caf`. Records the merge boundary and the next separate PRs. |
| [`gittarget-parent-empty-repository.md`](design/gittarget-parent-empty-repository.md) | **deferred.** An empty repository's default branch: the server reports it over protocol v2 but go-git discards it (upstream fix first); then a separate policy decision on which branch the first commit may create. Assuming `main` was considered and rejected. |
| [`gittarget-parent-observation.md`](design/gittarget-parent-observation.md) | **deferred.** Keep the parent visible after the write branch exists: availability first, ancestry only after measuring how often a shallow checkout can answer it. Informational; authorizes no branch mutation. |
Expand All @@ -148,7 +150,7 @@ so you never have to read a proposal to find out it already shipped.
| [`metrics-observability-plan.md`](design/metrics-observability-plan.md) | **built.** the canonical metrics document, rewritten 2026-09 and shipped in the same wave. Reads the product as two ingestion paths — object state through watch, authorship through audit — joining at the resolver, and gives each stage one counter with a bounded `outcome` so the flow and its loss paths are drawable. Records what three review rounds corrected, including two reversals worth not re-litigating: the fact-loss counters must NOT be merged (an eviction counts a fact, a trim gap an occurrence, a decode error a whole batch), and a cumulative counter cannot replace a current count, which is why `status.streams` and `status.retainedDocuments` stayed. The instrument inventory lives in [`interpreting-metrics.md`](interpreting-metrics.md), not here. Open: the Grafana JSON and alert rules |
| [`reconcile-triggering.md`](design/reconcile-triggering.md) | which controllers still fail to wake up |
| [`push-notification-and-reconcile-trigger.md`](design/push-notification-and-reconcile-trigger.md) | **part shipped, the rest is a menu.** Merges the old `inbound-push-notification.md` and `support-boundary/orchestrator-reconcile-trigger.md`, which argued two halves of one subject: the messages Reverser and the reconciler do not send each other. Part 1 is built and measured: the pre-push fetch is gone because the compare-and-swap push already reads the advertisement and a cycle that commits nothing still reaches it, carried by a three-flag invariant (`baseTrusted`/`worktreeDirty`/`replayRequired`, each bracketing a different span), a measured round-trip ledger, and a per-reason fetch counter. Part 2 is the two gaps: nothing tells an idle target its branch moved, and a refused write produces no commit so nothing reverts it. It also states **which side wins** (a captured API write beats Git silently; the cell worth protecting is the object a push changed that we hold no intent about) and why wiring a webhook to the `requestedAt` annotation is a correctness bug rather than a shortcut. Part 3 is six options with consequences, **doing nothing first**: a maximum age on `baseTrusted`, telling the reconciler after we push (which closes the API-first loop with **no Git-host webhook at all**), receiving a notification, taking the webhook over and forwarding it (so **we never have to work out which orchestrator object syncs a folder** — neither Argo CD nor Flux derives that mapping either), holding the push as an optimization that cannot be a barrier, triggering a reconcile to revert a refused edit, and **pushing an empty commit**, which closes that same gap for nothing: a new SHA defeats Argo CD's `selfHeal: false` skip legitimately (the skip is gated on having already synced *this revision*) and gives Flux a new artifact revision, so neither needs us to name an object, patch one, or hold an Argo sync authority. Its costs are a branch-wide blast radius and what it does under `prune: true`. Option 8 hands the action to the installer instead, and records that **executing a script mounted from a ConfigMap is declined**: it is arbitrary code execution as a process holding Git credentials, SSH and age keys and remote kubeconfigs, and Argo CD removed exactly that pattern in 2.8 in favour of a sidecar. The seam is a structured Event carrying the refused object and reason, which the existing `EventRecorder` makes nearly free. Options 7 and 8 together are why option 6, and the ownership interpreters it needs, may never be built |
| [`push-cooldown.md`](design/push-cooldown.md) | **design, open.** Whether the fixed `5s` push cooldown still earns its place now that the commit window exists. For ONE author writing one target it is provably inert at the defaults, because the window is a rolling silence timer of the same length. But there is only one open window per worker, so a second identity finalizes it on arrival: under alternating authors the silence timer never fires and the cooldown is the only thing batching, which makes a multi-user attributed cluster the normal case rather than the exception. Prices what removal would delete (about thirty lines and two fields) against what it would not touch (the retained-write, base-trust and replay machinery, which exists for push failure), and concludes that the unambiguous gap is the missing failure backoff rather than the cooldown itself |
| [`push-cooldown.md`](design/push-cooldown.md) | **partly built.** Whether the fixed `5s` push cooldown still earns its place now that the commit window exists. Ordinary timer closures already space one author writing one target, but identity changes, saves with shorter timers and buffer-limit closures commit sooner, and serialization does not batch queued commits, so the cooldown is what batches them. Option C, the failure backoff, is built in #412 and shares the push timer; removing the success wait would now remove only `lastPushAt` and its branch, so that decision waits on the event-loop measurements in §9 |
| [`multi-source-audit-ingress-hardening.md`](design/multi-source-audit-ingress-hardening.md) | how independent sources authenticate to a named audit route, when annotation routing is trustworthy, and how multi-provider ingestion remains fair |
| [`release-image-reuse-plan.md`](design/release-image-reuse-plan.md) | PRs 2–5 unstarted |
| [`e2e-coverage-gaps-and-improvements-plan.md`](design/e2e-coverage-gaps-and-improvements-plan.md) | tests A/B/C still proposals |
Expand Down Expand Up @@ -197,8 +199,10 @@ than install manifests. What changed is that they are now checked.
[`idea-application-editing.md`](future/idea-application-editing.md) is where the
whole edit-through-the-API workstream started, and still holds the branch/session
grouping strategies nothing else covers.
[`ha-gittarget-distribution-plan.md`](future/ha-gittarget-distribution-plan.md) is
the HA plan `architecture.md` cites three times (and the reason Redis is required).
[`ha-gittarget-distribution-plan.md`](future/ha-gittarget-distribution-plan.md) owns the durable
journal, publication recovery, outage retention, and persistence/HA rollout. The branch worker
event model owns execution semantics and the Git operation-deadline fix; its publication retry
shipped in #412.
[`least-privilege-remaining-work.md`](future/least-privilege-remaining-work.md) has
three open RBAC items.
[`config-surface-for-a-structured-repository.md`](future/config-surface-for-a-structured-repository.md)
Expand Down
26 changes: 6 additions & 20 deletions docs/TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,15 @@ PR #404 is merged; these need fix PRs against main, in this order. (Its CommitRe
finding is fixed in #407: the controller no longer fails a request the worker holds.)

The two `CommitEmpty` findings (a suspended target reporting `Ready=True` with no commit, and a
record made while the render-fidelity gate was closed) are fixed in `fix/branch-worker-write-gates`:
record made while the render-fidelity gate was closed) are fixed in #411:
a request on a target that may not be written fails, whatever path ends it
(`internal/git/write_gate.go`).

**Next:** retry progress on quiet branches. A failed push stops the push timer and the retained work
waits for another commit (`pushPending` in `internal/git/branch_worker.go`); parent recovery drives
itself, and general publication recovery needs the same guarantee
([`docs/design/push-cooldown.md`](design/push-cooldown.md) §7 option C).

**Before the next release** (docs only):

- [ ] `docs/UPGRADING.md`, the v1alpha3 window entry:
- step 1: tell readers to suspend the Flux or Argo source of a GitOps-managed GitTarget (or remove
the field there) until step 4; otherwise that tool re-applies the string `commit.window` and
breaks LIST;
- the Was/Is table needs a row for the chart value `quickstart.gitTarget.commit.window`, now an
object;
- "a removed `closeDelay` is not a hazard" is too strong: the API server prunes it silently for
programmatic clients (see `duration_fields_admission_test.go`).
- [ ] Descriptions that still say a save "closes the window now": the CommitRequest godoc and CRD
description, `docs/configuration.md` (lines 14, 32, 123), and the GitTarget `Commit` godoc, which
also omits the 1m `maxDuration` default. `docs/spec/commitrequest-design.md:126` still says
`NoOpenWindow` and "close deadline".
Retry progress on quiet branches is fixed: a failed publication schedules its own bounded retry
(`internal/git/publication_retry.go`, [`push-cooldown.md`](design/push-cooldown.md) §7 option C).
Still open: when a rebuild's fetch fails, the open window is dropped, and unless the cause was a
missing parent nothing records the dropped scope or asks for a snapshot, so those writes wait for
the next resync.

**Later, non-blocking:**

Expand Down
Loading
Loading