fix(repositories): declare org-enforced signoff so updates can apply - #136
Conversation
Seven of the twenty Repository resources have been failing every update with 422 "Commit signoff is enforced by the organization and cannot be disabled", so declared repository configuration has not reached GitHub for them. The accepted explanation was that the field is rejected whenever it appears in an update, so #128 removed the declaration entirely. The live cluster shows the opposite. upjet builds the Terraform configuration from forProvider; an absent optional bool takes the provider's zero value of false; false against a live true is a permanent diff, so the payload carries web_commit_signoff_required: false and GitHub rejects it. The error names disabling, not presence. Declaring the live value leaves nothing to diff, so Terraform omits the field from the payload and the update applies. Measured, at two-minute resolution: 2026-07-27T04:33Z #125 merges, shared patch still declares the field 2026-07-27T04:34:5x nine write-enabled repos record LastAsyncOperation=Success 2026-07-27T14:42Z #128 removes the declaration 2026-07-27T14:44:4x the same repos begin recording AsyncUpdateFailure (422) Ten repositories still reconcile today only because the provider had already late-initialized the value into their spec, where Flux's removal could not reach it. The seven that never accumulated that residue are exactly the seven failing, and six of them differ from a working repository by this one field. So the write path currently depends on undeclared residue rather than on anything in this repository. platform-tenant-template is the one active resource holding no adopted values, so it cannot fall back on a previously observed visibility once updates apply again; its live value is pinned explicitly. tests/repository-update-policy.sh now pins the inverted invariant. Fixes #112 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 49 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe shared repository Kustomize patch now sets Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (4 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 46-47: Clarify the LateInitialize ownership guidance in AGENTS.md
at lines 46-47: only fields already present in forProvider during adoption may
remain provider-owned; newly observed fields must not be copied after
LateInitialize is removed. Apply the same clarification to
deploy/repositories/agent-plugins.yaml lines 12-13 and
deploy/repositories/agent-skills.yaml lines 13-14, preserving each file’s
existing context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f84ab481-b6c8-400d-a575-d4b3c224f799
📒 Files selected for processing (10)
AGENTS.mddeploy/archived-repositories/kustomization.yamldeploy/repositories/agent-plugins.yamldeploy/repositories/agent-skills.yamldeploy/repositories/aws.yamldeploy/repositories/kustomization.yamldeploy/repositories/kyverno-policies.yamldeploy/repositories/platform-tenant-template.yamldeploy/repositories/provider-upjet-unifi.yamltests/repository-update-policy.sh
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
deploy/**/*.{yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
deploy/**/*.{yaml,yml}: Manage GitHub organization, repository, team, and label configuration declaratively by editingdeploy/and shipping a PR; never write managed configuration throughgh apior the GitHub UI.
When adopting an existing GitHub resource, setcrossplane.io/external-nameto its live name and use a management policy that excludesDelete.
Files:
deploy/repositories/platform-tenant-template.yamldeploy/repositories/agent-plugins.yamldeploy/repositories/agent-skills.yamldeploy/repositories/provider-upjet-unifi.yamldeploy/archived-repositories/kustomization.yamldeploy/repositories/kyverno-policies.yamldeploy/repositories/aws.yamldeploy/repositories/kustomization.yaml
deploy/repositories/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
After adoption, active
Repositoryresources must useObserve/Create/UpdatewithoutLateInitialize; values enforced by the organization, includingwebCommitSignoffRequired, must be declared inforProviderat the enforced value and not seeded throughinitProvider.
Files:
deploy/repositories/platform-tenant-template.yamldeploy/repositories/agent-plugins.yamldeploy/repositories/agent-skills.yamldeploy/repositories/provider-upjet-unifi.yamldeploy/repositories/kyverno-policies.yamldeploy/repositories/aws.yamldeploy/repositories/kustomization.yaml
deploy/**/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Verify every new Crossplane resource's kind and field schema against the provider's published CRDs and generated examples; local CI cannot perform full CRD validation.
Files:
deploy/repositories/platform-tenant-template.yamldeploy/repositories/agent-plugins.yamldeploy/repositories/agent-skills.yamldeploy/repositories/provider-upjet-unifi.yamldeploy/archived-repositories/kustomization.yamldeploy/repositories/kyverno-policies.yamldeploy/repositories/aws.yamldeploy/repositories/kustomization.yaml
AGENTS.md
📄 CodeRabbit inference engine (CLAUDE.md)
Follow the instructions defined in
AGENTS.md.
Files:
AGENTS.md
**/AGENTS.md
📄 CodeRabbit inference engine (GEMINI.md)
Follow the coding guidelines and instructions defined in
AGENTS.md.
Files:
AGENTS.md
tests/*.sh
📄 CodeRabbit inference engine (AGENTS.md)
Before every PR, run the required validation checks: Kustomize build, admin-team policy, declarative coverage, fail-closed label coverage, repository update policy, and release-contract tests.
Files:
tests/repository-update-policy.sh
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: devantler-tech/.github
Timestamp: 2026-08-06T01:20:36.110Z
Learning: Use GitHub Issues as the roadmap of record, triage incoming issues into the roadmap structure, and have implementation PRs use `Fixes `#N``.
📚 Learning: 2026-07-27T11:40:06.755Z
Learnt from: devantler
Repo: devantler-tech/.github PR: 127
File: deploy/repositories/agent-plugins.yaml:14-15
Timestamp: 2026-07-27T11:40:06.755Z
Learning: When reviewing Crossplane GitHub `Repository` resources under `deploy/repositories/*.yaml`, note that removing `LateInitialize` prevents newly observed/undeclared fields from being copied into `spec.forProvider`. As a result, those newly added settings become unmanaged until explicitly declared. However, fields that were late-initialized before this policy change may already exist in `spec.forProvider`; they are then owned by the provider field manager (not Flux `kustomize-controller`) and may remain in provider update payloads. Require comments to clearly distinguish these two cases so reviewers know whether a field is newly unmanaged vs. pre-existing provider-owned data.
Applied to files:
deploy/repositories/platform-tenant-template.yamldeploy/repositories/agent-plugins.yamldeploy/repositories/agent-skills.yamldeploy/repositories/provider-upjet-unifi.yamldeploy/repositories/kyverno-policies.yamldeploy/repositories/aws.yamldeploy/repositories/kustomization.yaml
📚 Learning: 2026-07-10T14:42:56.622Z
Learnt from: devantler
Repo: devantler-tech/.github PR: 90
File: deploy/archived-repositories/kustomization.yaml:11-16
Timestamp: 2026-07-10T14:42:56.622Z
Learning: In this repo’s deployment setup, `deploy/` kustomize manifests are rendered unconditionally (no feature-flag/conditional-render/apply primitive) and Flux applies from `main`. For irreversible or ordering-sensitive rollouts in `deploy/` (e.g., changes under `deploy/archived-repositories/` that require prerequisites handled in separate PRs), reviewers should not expect a mechanical CI guard. Instead, require the PR to remain in draft until the required prerequisite one-click actions (e.g., org `required-workflow` ruleset repointing) are completed, with the required rollout ordering explicitly documented in the PR body (per the documented bold line). Only then should the PR be promoted/merged.
Applied to files:
deploy/archived-repositories/kustomization.yamldeploy/repositories/kustomization.yaml
🔇 Additional comments (10)
deploy/repositories/platform-tenant-template.yaml (1)
20-25: LGTM!deploy/repositories/kustomization.yaml (2)
39-51: LGTM!
85-87: 🎯 Functional CorrectnessField usage matches the provider version.
spec.forProvider.webCommitSignoffRequired: trueis supported in the provider-upjet-githubrepo.github.m.upbound.io/v1alpha1Repositoryschema and generated examples use it directly.deploy/archived-repositories/kustomization.yaml (1)
2-4: LGTM!deploy/repositories/aws.yaml (1)
26-27: LGTM!deploy/repositories/kyverno-policies.yaml (1)
27-28: LGTM!deploy/repositories/provider-upjet-unifi.yaml (1)
22-23: LGTM!tests/repository-update-policy.sh (2)
61-80: LGTM!Also applies to: 94-107
81-92: 🎯 Functional CorrectnessNo change needed for the active repository filter.
deploy/archived-repositories/only includesreusable-workflows, and it sets.spec.forProvider.archived: true, so archived resources are already covered by the existing filter.> Likely an incorrect or invalid review comment.AGENTS.md (1)
48-55: LGTM!
CodeRabbit: the previous wording let 'late-initialized values land in forProvider' read as an ongoing behaviour. Once LateInitialize is removed no newly observed field is copied in again, which is precisely why a resource adopted without a field can never acquire it and the config must supply it.
@coderabbitai review |
|
|
CodeRabbit is rate limited at this head (commit status For context, CodeRabbit did serve the previous head and its one finding is fixed and resolved; this head differs from that one only by the wording fix it asked for. @codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Codex reports its code-review usage limit is reached at this head, so the loop advances to the third lane. Requesting Bugbot next (bare trigger in the following comment — it exact-matches the whole comment body, so the disclosure has to sit here instead of inline). |
|
@cursor review |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_9b91cd40-6d43-4640-a835-0cf27bcd4d59) |
devantler
left a comment
devantler
left a comment
There was a problem hiding this comment.
🤖 Generated by the Agentic Engineer
Self-review (fallback — CodeRabbit, Codex and Cursor Bugbot unavailable)
Reviewed commit: 058c81997ac82fc191afc2540cccb65ed225e518
Per-lane evidence at this exact head, checked directly on this PR (not inferred from a portfolio digest):
| Lane | Evidence | When |
|---|---|---|
| CodeRabbit | commit status success / description Review rate limited; reply comment "Action not completed — Review rate limited". No retry window stated. |
01:30:50Z |
| Codex | connector comment: "You have reached your Codex usage limits for code reviews." Account-level; no window. | 01:32:05Z |
| Cursor Bugbot | check-run conclusion=neutral with output.title=Error — the run never started (completed 2s after start, 0 inline comments). |
01:33:59Z |
CodeRabbit did serve the previous head (d4d0e859) and raised one 🟡 Minor, which is fixed and its thread resolved. This head differs from that reviewed head only by the wording change CodeRabbit asked for.
What I checked
Blast radius. Rendered deploy/ before vs after is exactly 19 × webCommitSignoffRequired: true plus 1 × visibility: public, with zero removals. For the ten repositories that currently reconcile, the declared value is identical to what their live spec already holds, so the object content does not change — only field ownership moves from the provider to the applier. The seven that fail are the only ones whose rendered state actually changes.
The value is correct for every target. Checked against the GitHub API directly rather than Crossplane's cached view: all seven failing repositories report web_commit_signoff_required=true live, so the declared value is a zero-diff match in every case.
Test guards. RED proved first (the new assertion named all 19 against the unfixed config), then GREEN. Both guards ablated individually: flipping the declared value true→false fired the missing-signoff assertion; adding an initProvider seed fired the second. Both null branches are exercised — 19 resources have no initProvider and the suite passes without erroring.
No coverage lost. The previous test forbade the field in forProvider or initProvider. The replacement still forbids initProvider seeding and additionally requires the correct forProvider value, so the enforced surface is strictly larger, not smaller.
Archived exclusion is live, not vacuous. reusable-workflows renders with archived: true and no signoff, and is correctly excluded — so the filter is doing real work rather than passing by absence.
Security direction. This pins a security control declaratively that was previously only org-enforced; if the org setting were ever relaxed, the repository-level declaration would keep signoff on. The one visibility pin fixes a repository at its current public value so restoring the write path cannot move it. Nothing here relaxes a control.
Not verified here, by design
Whether the seven repositories actually flip to Synced can only be observed after the release tag publishes the OCI artifact and Flux reconciles it. That is stated in the PR body and will be recorded on #112 with a live read; this PR does not close the issue on merge alone, which is what caused it to be reopened twice before.
Verdict: no P0/P1 findings.
ReadinessProgrammatically tested. All six baseline validate commands pass; CI 13/13 green at Reviewed. CodeRabbit served the previous head
Exercised. This change has no runtime surface that can be exercised before merge — the manifests are consumed on-cluster by the
The behavioural proof — the seven flipping from |
1 participant
Why
Declarative repository configuration has not been reaching GitHub. Seven of the twenty managed repositories reject every settings update with a 422, so anything declared for them — description, topics, merge policy, visibility — silently never applies. The ten that still work do so only by accident of history, not because the config makes them work, which means the same failure reaches any newly-declared repository. That is how this surfaced:
doggy-countdownwas added and was broken from the moment it was created.Two earlier attempts (#127, #128) closed the tracking issue and were reopened within minutes each time, because both were built on the same wrong explanation of the cause.
What
Declares the signoff setting at the value the organization already enforces, instead of leaving it unset. Leaving it unset is what was generating the rejected update; declaring it removes the conflict, and the rest of the update then applies normally.
The live cluster records the switch at two-minute resolution: with the setting declared, nine repositories completed real updates; two minutes after it was removed, those same repositories started failing, and seven were still failing today.
One repository (
platform-tenant-template) holds no previously-adopted settings, so restoring updates there could otherwise have changed its visibility — its current value is pinned explicitly so it cannot move.The CI check that had locked in the wrong rule now enforces the correct one, so this cannot regress.
Verification after merge: the seven failing repositories should report a successful sync rather than the 422. That is a live read, recorded on the issue — this does not close on merge alone.
Fixes #112