fix(commitrequest): keep the creating fact past a no-op write, and order the 0.54 upgrade

api/v1alpha3/commitrequest_schema_test.go

@@ -19,7 +19,7 @@ import (
 // TestCommitRequestSchemaRevision: raise CommitRequestSchemaRevision, and the revision line, and add
 // the new hash here.
 var commitRequestSchemaHashes = map[int]string{
-	1: "ea99cba6116e0f562c0be931c13d6580eabdc2e7ba32d486d20a0a5029163e99",
+	1: "395df39c86bc4c0350eb2c476dd0ae02db2a7513f03962103416123b0cbb5476",
 }
 
 // commitRequestRevisionLine is the last line of the kind's description.

api/v1alpha3/commitrequest_types.go

@@ -228,6 +228,9 @@ type CommitRequestStatus struct {
 	//   holds the saved objects.
 	// - False, reason CommitDropped: the branch no longer holds the commit;
 	//   its history was rewritten after the push. Only this is False.
+	// - Unknown, reason TimedOut: the apply did not settle within 10 minutes
+	//   of Ready, so the request stopped following it. The commit stays
+	//   pushed; the message says where the apply last stood.
 	// - Unknown, reason Lost: GitOps Reverser restarted before it knew.
 	// +optional
 	// +listType=map

charts/gitops-reverser/README.md

@@ -345,6 +345,11 @@ curl http://localhost:8080/metrics
 
 ## Upgrading
 
+Before upgrading, read the [upgrade guide](../../docs/UPGRADING.md) entries for every release you
+cross, and follow their steps in order. Upgrading from 0.54.0 starts with
+[draining the `CommitRequest`s](../../docs/UPGRADING.md#upgrading-from-0540-drain-the-commitrequests-first).
+Then apply the new release's `crds.yaml` ([CRD lifecycle](#crd-lifecycle)), and upgrade.
+
 ### Standard Upgrade
 
 ```bash

config/crd/bases/configbutler.ai_commitrequests.yaml

@@ -289,6 +289,9 @@ spec:
                     holds the saved objects.
                   - False, reason CommitDropped: the branch no longer holds the commit;
                     its history was rewritten after the push. Only this is False.
+                  - Unknown, reason TimedOut: the apply did not settle within 10 minutes
+                    of Ready, so the request stopped following it. The commit stays
+                    pushed; the message says where the apply last stood.
                   - Unknown, reason Lost: GitOps Reverser restarted before it knew.
                 items:
                   description: Condition contains details for one aspect of the current

docs/UPGRADING.md

@@ -7,6 +7,53 @@ guidance that the changelog's breaking-change entries link to.
 We are pre-1.0, so breaking changes bump the **minor** version (release-please is configured with
 `bump-minor-pre-major`) rather than the major. Read the relevant entry before upgrading across it.
 
+## Upgrading from 0.54.0: drain the `CommitRequest`s first
+
+Existing providers, branches, targets and claims keep running across this upgrade; none of the
+0.53.0 recreation [below](#upgrading-from-0530-recreate-the-connection-objects-in-one-pass) applies.
+What needs ordering is the `CommitRequest`s that are in progress: 0.54.0 wrote progress conditions
+this release does not take over, so a request still open when it starts can stay open for good.
+
+1. Stop creating `CommitRequest`s: pause the automation and people that create them.
+2. Wait for every request to end. This lists the ones that have not, and should come back empty
+   once each has passed its `spec.window` bounds:
+
+   ```bash
+   kubectl get commitrequests -A -o json | jq -r '.items[]
+     | select([.status.conditions[]? | select((.type == "Ready" or .type == "Stalled")
+         and .status == "True")] | length == 0)
+     | "\(.metadata.namespace)/\(.metadata.name)"'
+   ```
+
+3. Adjust the audit policy and values, as the [entry below](#the-admission-webhook-is-gone-a-commitrequests-author-comes-from-audit)
+   describes: `commitrequests` `create` and `patch` at `RequestResponse` level, and no
+   `servers.admission` or `--admission-webhook*` settings.
+4. Apply the release's `crds.yaml`, then upgrade, as the
+   [chart's upgrade steps](../charts/gitops-reverser/README.md#upgrading) describe.
+5. Remove the admission webhook's leftovers.
+   - **Helm** deletes the `ValidatingWebhookConfiguration` and the `Certificate` it rendered, but
+     not the Secret cert-manager wrote for that certificate. Delete it:
+
+     ```bash
+     kubectl -n gitops-reverser delete secret gitops-reverser-admission-server-cert --ignore-not-found
+     ```
+
+   - **`install.yaml`** is applied with `kubectl apply`, which removes nothing. Delete the three
+     resources yourself:
+
+     ```bash
+     kubectl delete validatingwebhookconfiguration gitops-reverser-validate-operator-types --ignore-not-found
+     kubectl -n gitops-reverser delete certificate gitops-reverser-admission-server-cert --ignore-not-found
+     kubectl -n gitops-reverser delete secret gitops-reverser-admission-server-cert --ignore-not-found
+     ```
+
+   The names are those of a release named `gitops-reverser` in namespace `gitops-reverser`; use
+   yours if they differ.
+6. Run the command from step 2 again. A request it still lists was open across the upgrade and may
+   never end; one made on the cluster GitOps Reverser runs in, for a `GitTarget` that mirrors another
+   cluster, certainly will not. Delete each, and create it again: on the cluster the `GitTarget`
+   mirrors, for such a `GitTarget`. Then resume creating requests.
+
 ## The admission webhook is gone; a `CommitRequest`'s author comes from audit
 
 GitOps Reverser installs no admission webhook. The chart no longer renders the
@@ -22,8 +69,10 @@ those that remain expire on their own.
   certificate with `servers.metrics.tls.enabled`. The default install needs no cert-manager.
 - **A request's author.** A `CommitRequest`'s author comes from its own audit event, as a write's
   does, and is reported in `status.author`. The `AuthorAttributed` condition is gone. Keep `create`
-  of `commitrequests` in your audit policy; the write catch-all of the
-  [tuned policy](../test/e2e/cluster/audit/policy.yaml) covers it. A request no audit event names, on
+  and `patch` of `commitrequests` in your audit policy at `RequestResponse` level: the author is
+  matched by the uid and resourceVersion in the response, which `Metadata` level leaves out, and
+  server-side apply creates with a `patch`. The write catch-all of the
+  [tuned policy](../test/e2e/cluster/audit/policy.yaml) covers both. A request no audit event names, on
   a cluster that sends audit events, ends `Ready=True` with reason `AuthorUnresolved` and saves
   nothing. With attribution off, a request names nobody, as it did with the webhook off.
 - **Status while it runs.** A `CommitRequest` in progress is `Ready=Unknown`, reason `Saving` until the

docs/architecture.md

@@ -172,8 +172,9 @@ the Flux `Kustomization` applier and the Argo CD `Application` applier, its auto
 
 **Redis/Valkey is optional but advised.** The default configured-author mode runs without it: a plain
 `helm install` comes up healthy and watches initialize from current state on startup. When an endpoint
-is configured, Redis stores cursors for reconnecting running watches and the coordination records used by
-CommitRequest author capture and HA. Attribution does not require it on its own: its facts travel on a
+is configured, Redis stores cursors for reconnecting running watches. A CommitRequest's author comes from
+the audit fact of the write that created it, like any write's, and needs no Redis record.
+Attribution does not require it on its own: its facts travel on a
 selectable transport, Redis Streams by default and an in-process ring with
 `--author-attribution-transport=memory`, which is refused with more than one replica. HA will require
 Redis as the shared store across replicas.

docs/attribution-setup-guide.md

@@ -120,10 +120,12 @@ other subresources. Keep manual scale writes if you want their authors recorded.
 The example includes Secret writes at `RequestResponse` level. To keep Secret bodies out of the
 webhook, exclude core `secrets` too. Those writes then have no audit evidence for attribution.
 
-Keep `create` of `commitrequests` in the `configbutler.ai` group covered, at `Metadata` level at
-least: a `CommitRequest`'s author comes from its own audit event, as a write's does. Without it a
-request names nobody, and cannot save the window of an author the audit names. The write catch-all
-covers it.
+Keep `create` and `patch` of `commitrequests` in the `configbutler.ai` group covered at
+`RequestResponse` level. A `CommitRequest`'s author comes from the audit event that created it,
+matched by the uid and resourceVersion in the event's `responseObject`. `Metadata` level leaves
+the response out, so the request cannot be matched. `patch` is there because server-side apply
+creates an object with a `patch`. Without the creating event, a request names nobody and cannot
+save the window of an author the audit names. The write catch-all covers both verbs.
 The audit policy controls evidence collection; `WatchRule` and `ClusterWatchRule` control what
 gets written to Git.
 

docs/configuration.md

@@ -3059,9 +3059,12 @@ separates work the worker already collected from work that reaches it afterward.
 write was made after the request: a write still held for its audit fact arrives later and lands in
 the next window.
 
-Competing requests from one author are served first come, first served. A window carries at most
-one request; the next waits for the next window. A re-sent request is recognized by its identity
-and restarts nothing: its deadlines and, for `Next`, its one close are fixed at first registration.
+Competing requests from one author are not queued. A window carries one message, so while an
+earlier request of the same author on the same `GitTarget` waits for a window or collects one, a
+later request is refused at once with [`SaveInProgress`](#its-status). A `Next` request is not
+refused by one that is collecting: it closes that window under the earlier request's message, then
+waits for its own. A re-sent request is recognized by its identity and restarts nothing: its
+deadlines and, for `Next`, its one close are fixed at first registration.
 
 #### Sizing the request window
 
@@ -3255,6 +3258,25 @@ request produced a pushed commit:
 A request is served again after GitOps Reverser restarts: the path that owns it finds its own
 claim, and resumes with the author it recorded.
 
+### How long a request lives
+
+A `CommitRequest` is a single save, not a standing order: every stage it passes through ends on its
+own, and the status says which limit ended it. To save again, create a new request.
+
+| Stage | Ends after | What the request then says |
+| --- | --- | --- |
+| Picked up, author named, waiting for a branch worker | `attachTimeout` + `maxDuration` + 2 minutes, counted from creation | `Ready=False`, `Stalled=True`, reason `FinalizeFailed`: the `GitTarget` did not start a branch worker, or the request was not acted on, within the safety window |
+| Waiting for a window (`WaitingForWindow`) | `attachTimeout` | `Ready=True`, reason `NoWindow` (see [When there is nothing to commit](#when-there-is-nothing-to-commit)) |
+| Collecting its window (`CollectingWindow`) | `maxDuration`, at most `5m` | the window closes and is committed |
+| Waiting for the push (`WaitingForPush`) | no limit of its own | stays `Ready=Unknown`; `Reconciling` says why the push waits |
+| Waiting for its apply, with `catchUp: AfterPush` | 10 minutes from `Ready` | `Applied=Unknown`, reason `TimedOut` (see [Applying a save at once](#applying-a-save-at-once)) |
+| Finished | 48 hours, by default | deleted (see [Finished requests are deleted after 48 hours](#finished-requests-are-deleted-after-48-hours)) |
+
+The push is the one wait without a limit, deliberately. Once the window is committed, the commit is
+going to the remote: failing the request while the worker still retries, every 10 seconds doubling
+to 5 minutes, would report a save as failed that then lands. A worker that stops before the push
+fails the request instead. A request that waits there needs the remote fixed, not a new request.
+
 ### Applying a save at once
 
 A save is in Git once it is pushed, and the cluster already runs it, so by default nothing applies
@@ -3272,10 +3294,13 @@ already at the tip needs none. The request reports the outcome in its `Applied`
 | `True` | `Applied` | the applier applied a revision that holds the commit; `status.appliedRevision` names it, as the applier reported it |
 | `True` | `NothingToApply`, `NoApplierClaim`, `ApplierGone` | no apply was needed: the request pushed no commit, or the `GitTarget` names no applier claim, or stopped naming the one asked. The cluster already holds what was saved |
 | `False` | `CommitDropped` | the branch no longer holds the commit: its history was rewritten after the push. The only `False` |
+| `Unknown` | `TimedOut` | the apply did not settle within 10 minutes of `Ready`, so the request stopped following it. The commit stays pushed, and the applier may still apply it; the message says where the apply last stood, and the applier claim's status reports later applies |
 | `Unknown` | `Lost` | GitOps Reverser restarted before it learnt how the apply went |
 
-Wait for `Applied=True` for "saved and applied"; `Ready=True` stays "saved". A finished request is
-deleted only after its `Applied` condition is settled.
+Wait for `Applied=True` for "saved and applied"; `Ready=True` stays "saved". A request follows its
+apply for 10 minutes at most: an applier that cannot catch up does not keep it open, and a request
+that ends `TimedOut` needs the applier looked at, not a new request. A finished request is deleted
+only after its `Applied` condition is settled.
 
 ### Finished requests are deleted after 48 hours
 

docs/design/commit-timing-surface.md

@@ -166,8 +166,10 @@ Two rules keep the edges honest:
   never closes a second window.
 - **`Next` needs time to wait.** CEL refuses `attach: Next` with `attachTimeout: 0s`: the save would
   close the author's window and give up in the same instant, collecting nothing.
-- **Competing saves are served first come, first served,** in the order the worker first registered
-  them. A window carries at most one save; the next waits for the next window.
+- **Competing saves are not queued.** A window carries one message, so a later save of the same
+  author on the same `GitTarget` is refused with `SaveInProgress` while the earlier one waits or
+  collects. A `Next` save closes a collecting window under the earlier message, then waits for its
+  own.
 
 What `Next` guarantees is narrower than its name suggests. It separates work the worker already
 collected from work that reaches the worker afterward. It cannot prove a write was made after the

docs/spec/commitrequest-design.md

@@ -102,9 +102,9 @@ A window is only ever opened by a write; a request attaches to one and never ope
 registration, the worker sets the attach deadline to registration plus `window.attachTimeout`, and
 repeated reconciles are idempotent: they restart nothing and, for `attach: Next`, close nothing again.
 With `CurrentOrNext` the request attaches to the author's window already open at registration, before
-any deadline is consulted, so `attachTimeout: 0s` can attach at all. Otherwise it waits, and waiting
-requests are served first come, first served. A request whose deadline has passed resolves and never
-takes a later window; when its deadline and a matching write are ready on the same wake, expiry wins.
+any deadline is consulted, so `attachTimeout: 0s` can attach at all. Otherwise it waits. A request
+whose deadline has passed resolves and never takes a later window; when its deadline and a matching
+write are ready on the same wake, expiry wins.
 
 On attach, the request's `idleTimeout` and `maxDuration` replace the window's timers: `maxDuration`
 runs from the attach, and so does the first idle interval. A waiting request gets a new window before
@@ -129,7 +129,9 @@ its first edits before they reach Git. A Git host's push notice leaves an attach
 Each request attaches
 to at most one window and cannot rename a finalized commit, including one waiting for push. A bundle
 provides no ordering guarantee. The wait does not reserve a transaction. Competing requests are
-served in registration order, first come, first served.
+not queued: a later request of the same author on the same `GitTarget` is refused with
+`SaveInProgress` while the earlier one waits or collects, except that a `Next` request closes a
+collecting window under the earlier message and then waits for its own.
 
 `spec.message` is literal, including template-like text and surrounding spaces. It is never parsed
 as a template, so a request author cannot execute one. Omission uses