fix(repositories): keep org-enforced signoff out of update payloads - #127
Conversation
The org enforces web commit signoff, and GitHub rejects an entire repository PATCH with 422 whenever web_commit_signoff_required appears in it, whatever value the field carries. The shared patch pinned it into forProvider, which is exactly the payload every update sends, so no declared setting could reach GitHub for any repository that had a pending diff. Move it to initProvider, which Crossplane applies only at creation, and drop LateInitialize from the active management policy so live-only values stop leaking back into forProvider and re-poisoning later updates. tests/repository-update-policy.sh pins all three invariants and runs in CI. Design and diagnosis from #120 (codex lane), rebased onto current main.
ascoachingogvaner and wedding-app declare `visibility: private` while both live repositories are public, and both host a deployed site. Restoring the update path would otherwise make the reconciler act on that declaration and turn two live public sites private, as a side effect of an unrelated fix. Park both on Observe until #123 settles which side is wrong. The update-policy test now pins the actual hazards — no LateInitialize, no Delete — instead of an exact policy cardinality, so a read-only repository satisfies it by construction.
…ally Replacing managementPolicies in the shared patch would have overridden every repository's own policy, promoting world-at-ruin from its deliberate Observe-only state and defeating the Observe-first adoption ladder for any repo added later. Remove LateInitialize from each Repository resource instead, leaving each repo's position on that ladder under its own file. DefaultBranch resources keep their policy; they are a different API and are not affected by the signoff rejection. ascoachingogvaner and wedding-app drop to Observe in their own files rather than through a name-matched exception patch.
The per-repo comments described LateInitialize adopting settings on every reconcile, which is no longer what happens. State the current contract instead: the shared patch supplies the org-wide merge policy, the file supplies what it declares, and anything in neither keeps the value adopted at first observation.
@coderabbitai review |
@codex review |
|
✅ Action performedReview finished.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99fc268f6d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
📝 WalkthroughWalkthroughRepository manifests remove Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 `@deploy/repositories/agent-plugins.yaml`:
- Around line 14-15: Clarify the repeated comments describing omitted repository
settings as unmanaged rather than implying Crossplane adopts or manages live
values. Update the wording at deploy/repositories/agent-plugins.yaml lines
14-15, deploy/repositories/agent-skills.yaml lines 15-16,
deploy/repositories/doggy-countdown.yaml lines 11-13,
deploy/repositories/dotnet-template.yaml lines 8-12,
deploy/repositories/fleet-gitops.yaml lines 8-13,
deploy/repositories/gitops-tenant-template.yaml lines 8-12,
deploy/repositories/go-template.yaml lines 8-12,
deploy/repositories/homebrew-tap.yaml lines 8-12, and
deploy/repositories/ksail.yaml lines 10-15 to state that settings absent from
both forProvider and the shared patch remain observed from live state without
being copied into or managed by the spec.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cdf5e835-06f9-4a0c-b90e-844bf8c7e35d
📒 Files selected for processing (22)
.github/workflows/ci.yamlAGENTS.mddeploy/repositories/agent-plugins.yamldeploy/repositories/agent-skills.yamldeploy/repositories/ascoachingogvaner.yamldeploy/repositories/aws.yamldeploy/repositories/doggy-countdown.yamldeploy/repositories/dotnet-template.yamldeploy/repositories/fleet-gitops.yamldeploy/repositories/gitops-tenant-template.yamldeploy/repositories/go-template.yamldeploy/repositories/homebrew-tap.yamldeploy/repositories/ksail.yamldeploy/repositories/kustomization.yamldeploy/repositories/kyverno-policies.yamldeploy/repositories/maintenance.yamldeploy/repositories/platform-template.yamldeploy/repositories/platform.yamldeploy/repositories/provider-upjet-unifi.yamldeploy/repositories/unifi.yamldeploy/repositories/wedding-app.yamltests/repository-update-policy.sh
💤 Files with no reviewable changes (3)
- deploy/repositories/provider-upjet-unifi.yaml
- deploy/repositories/kyverno-policies.yaml
- deploy/repositories/aws.yaml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
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 through the GitHub UI orgh api. Reading is allowed, and applying an existing label is triage rather than configuration.
When adopting an existing GitHub resource, setcrossplane.io/external-nameto the live name and use a management policy that excludesDeleteto prevent recreation or deletion.
Verify every Crossplane kind and field against the authoritative provider CRDs and generated examples; local schema validation is unavailable.
Preserve the documenteddeploy/layout: repositories, teams, team memberships, team-repositories, labels, provider configuration, external secret, and top-level kustomization belong in their designated paths.
Ensure the top-leveldeploy/kustomization.yamlwires all deployment resources and the shared repository-settings patch.
Before every PR, runkubectl kustomize deploy/,bash tests/admin-team-policy.sh,bash tests/declarative-coverage.sh, andbash tests/repository-update-policy.sh; all must pass.
Crossplane CRs must be schema-checked against the provider’s published CRDs because CI only runskubectl kustomizeand does not validate CRDs locally.
Files:
deploy/repositories/ksail.yamldeploy/repositories/platform-template.yamldeploy/repositories/unifi.yamldeploy/repositories/go-template.yamldeploy/repositories/fleet-gitops.yamldeploy/repositories/dotnet-template.yamldeploy/repositories/gitops-tenant-template.yamldeploy/repositories/doggy-countdown.yamldeploy/repositories/agent-plugins.yamldeploy/repositories/ascoachingogvaner.yamldeploy/repositories/wedding-app.yamldeploy/repositories/homebrew-tap.yamldeploy/repositories/agent-skills.yamldeploy/repositories/platform.yamldeploy/repositories/kustomization.yamldeploy/repositories/maintenance.yaml
deploy/repositories/**/*.{yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
deploy/repositories/**/*.{yaml,yml}: For an adopted activeRepository, useObserve/Create/UpdatewithoutLateInitialize; keep late-initialized values out of subsequent update payloads.
Put org-enforced creation defaults such aswebCommitSignoffRequiredininitProvider, notforProvider, because GitHub rejects that field in update PATCH requests.
Files:
deploy/repositories/ksail.yamldeploy/repositories/platform-template.yamldeploy/repositories/unifi.yamldeploy/repositories/go-template.yamldeploy/repositories/fleet-gitops.yamldeploy/repositories/dotnet-template.yamldeploy/repositories/gitops-tenant-template.yamldeploy/repositories/doggy-countdown.yamldeploy/repositories/agent-plugins.yamldeploy/repositories/ascoachingogvaner.yamldeploy/repositories/wedding-app.yamldeploy/repositories/homebrew-tap.yamldeploy/repositories/agent-skills.yamldeploy/repositories/platform.yamldeploy/repositories/kustomization.yamldeploy/repositories/maintenance.yaml
.github/workflows/ci.yaml
📄 CodeRabbit inference engine (AGENTS.md)
The CI workflow must provide the PR-time
CI - Required Checksgate using the four mandated validation commands.
Files:
.github/workflows/ci.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.Keep this repository’s
AGENTS.mdsynchronized with the actualdeploy/layout and shared engineering contract; do not add a redundant.github/copilot-instructions.md.
Files:
AGENTS.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: devantler-tech/.github
Timestamp: 2026-07-27T08:39:36.686Z
Learning: Use GitHub Issues as the roadmap of record; triage incoming work into the epic and roadmap-labelled children, and use `Fixes `#N`` in implementing PRs.
Learnt from: CR
Repo: devantler-tech/.github
Timestamp: 2026-07-27T08:39:36.686Z
Learning: Bring newly discovered repositories or out-of-band organization, repository, or team changes under declarative management through Observe-first roadmap/enhancement work, never by applying a UI fix.
📚 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/repositories/kustomization.yaml
🪛 LanguageTool
AGENTS.md
[style] ~90-~90: Consider an alternative for the overused word “exactly”.
Context: ...invariants ``` Those four commands are exactly what ci.yaml runs. `kubectl` (with b...
(EXACTLY_PRECISELY)
🔇 Additional comments (11)
deploy/repositories/ascoachingogvaner.yaml (1)
8-16: LGTM!deploy/repositories/maintenance.yaml (1)
1-6: LGTM!Also applies to: 14-17
deploy/repositories/platform-template.yaml (1)
8-16: LGTM!deploy/repositories/platform.yaml (1)
10-19: LGTM!deploy/repositories/unifi.yaml (1)
8-16: LGTM!deploy/repositories/wedding-app.yaml (1)
8-16: LGTM!deploy/repositories/kustomization.yaml (2)
4-14: LGTM!
60-85: 📐 Maintainability & Code QualitySchema-check the Repository
initProvider.webCommitSignoffRequiredpath before merge.This repository does not declare the
provider-upjet-githubpackage version or store the published CRD, but the code comment describeswebCommitSignoffRequiredas create-only to avoid GitHub 422 updates. Confirm the installed provider’srepo.github.m.upbound.io/RepositoryCRD and generated example exposespec.initProvider.webCommitSignoffRequired; otherwise this shared patch can add an unknown/unsupported field into deploy configs.tests/repository-update-policy.sh (1)
1-86: LGTM!.github/workflows/ci.yaml (1)
38-39: LGTM!AGENTS.md (1)
44-49: LGTM!Also applies to: 87-90
Two review findings, both real. The update-policy check rejected LateInitialize outright, which fails CI for a repository sitting in this repo's own documented Observe-first adoption phase — Observe+LateInitialize sends no update, so it cannot produce a bad payload. The hazard is pairing LateInitialize with Update; assert that instead. The shared initProvider seed was a JSON Patch `add` on the map itself, which replaces it wholesale and would silently drop any create-only setting a repository declared for itself. Use a merge patch, which creates the map when absent and merges into it when present. Also states what an undeclared setting actually does now: it is unmanaged, rather than re-adopted on every reconcile.
@coderabbitai review |
|
✅ Action performedReview finished.
|
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 `@deploy/repositories/doggy-countdown.yaml`:
- Around line 12-14: Clarify the comments describing post-LateInitialize
behavior: newly observed undeclared fields are unmanaged, while fields
late-initialized before this change may remain in spec.forProvider as
provider-owned values and continue reaching update payloads. Apply this explicit
legacy-behavior wording at deploy/repositories/doggy-countdown.yaml lines 12-14,
deploy/repositories/dotnet-template.yaml lines 11-13,
deploy/repositories/fleet-gitops.yaml lines 12-14,
deploy/repositories/go-template.yaml lines 11-13,
deploy/repositories/homebrew-tap.yaml lines 10-12,
deploy/repositories/platform-template.yaml lines 11-13,
deploy/repositories/platform.yaml lines 14-16, and
deploy/repositories/unifi.yaml lines 11-13.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9657d79c-4048-4044-ab7b-be5a33af3e1c
📒 Files selected for processing (11)
deploy/repositories/doggy-countdown.yamldeploy/repositories/dotnet-template.yamldeploy/repositories/fleet-gitops.yamldeploy/repositories/gitops-tenant-template.yamldeploy/repositories/go-template.yamldeploy/repositories/homebrew-tap.yamldeploy/repositories/kustomization.yamldeploy/repositories/platform-template.yamldeploy/repositories/platform.yamldeploy/repositories/unifi.yamltests/repository-update-policy.sh
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
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.
Verify every new Crossplane resource's kind and field schema against the authoritative provider CRDs and generated examples; local schema validation is unavailable.
Files:
deploy/repositories/dotnet-template.yamldeploy/repositories/platform.yamldeploy/repositories/go-template.yamldeploy/repositories/doggy-countdown.yamldeploy/repositories/gitops-tenant-template.yamldeploy/repositories/platform-template.yamldeploy/repositories/fleet-gitops.yamldeploy/repositories/unifi.yamldeploy/repositories/homebrew-tap.yamldeploy/repositories/kustomization.yaml
deploy/{repositories,teams,labels}/**/*.{yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
When adopting an existing repository, issue-label set, or team, use the live name in the
crossplane.io/external-nameannotation and a management policy excludingDeleteto observe/late-initialize without recreate or deletion.
Files:
deploy/repositories/dotnet-template.yamldeploy/repositories/platform.yamldeploy/repositories/go-template.yamldeploy/repositories/doggy-countdown.yamldeploy/repositories/gitops-tenant-template.yamldeploy/repositories/platform-template.yamldeploy/repositories/fleet-gitops.yamldeploy/repositories/unifi.yamldeploy/repositories/homebrew-tap.yamldeploy/repositories/kustomization.yaml
deploy/repositories/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
deploy/repositories/*.yaml: After adopting aRepository, useObserve/Create/UpdatewithoutLateInitialize; keep late-initialized values inforProvider, and put create-only defaults such aswebCommitSignoffRequiredininitProviderso they are not sent in update PATCH requests.
Define oneRepositoryresource per managed repository file.
Files:
deploy/repositories/dotnet-template.yamldeploy/repositories/platform.yamldeploy/repositories/go-template.yamldeploy/repositories/doggy-countdown.yamldeploy/repositories/gitops-tenant-template.yamldeploy/repositories/platform-template.yamldeploy/repositories/fleet-gitops.yamldeploy/repositories/unifi.yamldeploy/repositories/homebrew-tap.yamldeploy/repositories/kustomization.yaml
tests/*.sh
📄 CodeRabbit inference engine (AGENTS.md)
Before every PR, run the required validation commands:
kubectl kustomize deploy/,bash tests/admin-team-policy.sh,bash tests/declarative-coverage.sh, andbash tests/repository-update-policy.sh.
Files:
tests/repository-update-policy.sh
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: devantler-tech/.github
Timestamp: 2026-07-27T11:39:43.726Z
Learning: Use GitHub Issues as the roadmap of record; triage incoming issues into epic `#56` and its `roadmap`-labelled children, and use `Fixes `#N`` in implementing pull requests.
Learnt from: CR
Repo: devantler-tech/.github
Timestamp: 2026-07-27T11:39:43.726Z
Learning: When adopting existing GitHub resources or responding to drift, use an Observe-first declarative change in `deploy/`; never fix managed drift directly through the GitHub UI or imperative API writes.
📚 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/dotnet-template.yamldeploy/repositories/platform.yamldeploy/repositories/go-template.yamldeploy/repositories/doggy-countdown.yamldeploy/repositories/gitops-tenant-template.yamldeploy/repositories/platform-template.yamldeploy/repositories/fleet-gitops.yamldeploy/repositories/unifi.yamldeploy/repositories/homebrew-tap.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/repositories/kustomization.yaml
🔇 Additional comments (1)
deploy/repositories/gitops-tenant-template.yaml (1)
8-17: LGTM!
| # config is AUTHORITATIVE for it. Settings in neither | ||
| # are unmanaged: nothing new is copied into the spec, and values adopted | ||
| # earlier stay. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify the legacy late-initialized-field behavior.
“Values adopted earlier stay” obscures the material distinction: newly observed undeclared fields are unmanaged, while fields late-initialized before this change may already remain in spec.forProvider, be provider-owned, and still reach update payloads.
deploy/repositories/doggy-countdown.yaml#L12-L14: state the legacy provider-owned-field behavior explicitly.deploy/repositories/dotnet-template.yaml#L11-L13: state the legacy provider-owned-field behavior explicitly.deploy/repositories/fleet-gitops.yaml#L12-L14: state the legacy provider-owned-field behavior explicitly.deploy/repositories/go-template.yaml#L11-L13: state the legacy provider-owned-field behavior explicitly.deploy/repositories/homebrew-tap.yaml#L10-L12: state the legacy provider-owned-field behavior explicitly.deploy/repositories/platform-template.yaml#L11-L13: state the legacy provider-owned-field behavior explicitly.deploy/repositories/platform.yaml#L14-L16: state the legacy provider-owned-field behavior explicitly.deploy/repositories/unifi.yaml#L11-L13: state the legacy provider-owned-field behavior explicitly.
Based on learnings: removal of LateInitialize affects newly observed fields differently from previously provider-owned values.
📍 Affects 8 files
deploy/repositories/doggy-countdown.yaml#L12-L14(this comment)deploy/repositories/dotnet-template.yaml#L11-L13deploy/repositories/fleet-gitops.yaml#L12-L14deploy/repositories/go-template.yaml#L11-L13deploy/repositories/homebrew-tap.yaml#L10-L12deploy/repositories/platform-template.yaml#L11-L13deploy/repositories/platform.yaml#L14-L16deploy/repositories/unifi.yaml#L11-L13
🤖 Prompt for 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.
In `@deploy/repositories/doggy-countdown.yaml` around lines 12 - 14, Clarify the
comments describing post-LateInitialize behavior: newly observed undeclared
fields are unmanaged, while fields late-initialized before this change may
remain in spec.forProvider as provider-owned values and continue reaching update
payloads. Apply this explicit legacy-behavior wording at
deploy/repositories/doggy-countdown.yaml lines 12-14,
deploy/repositories/dotnet-template.yaml lines 11-13,
deploy/repositories/fleet-gitops.yaml lines 12-14,
deploy/repositories/go-template.yaml lines 11-13,
deploy/repositories/homebrew-tap.yaml lines 10-12,
deploy/repositories/platform-template.yaml lines 11-13,
deploy/repositories/platform.yaml lines 14-16, and
deploy/repositories/unifi.yaml lines 11-13.
Source: Learnings
Readiness evidence at Programmatically tested. Reviewed. Codex and CodeRabbit both reviewed; three findings, all fixed and their threads resolved. Tried and evaluated as a user. The mechanism this fix depends on is that Flux's server-side apply That is the whole intended state transition, observed rather than reasoned about. Also confirmed the What is not yet proven, and cannot be pre-merge: that topics actually appear on GitHub. That needs |
1 participant
Why
None of the repository settings we declare in this repo could actually reach GitHub for any repo that
had something to apply. GitHub rejects an entire repository update when it mentions the commit-signoff
setting the organization already enforces, and our shared config put that setting into every update.
The visible cost:
agent-pluginsandagent-skillseach declare a list of topics so people can findthem, and both still show no topics at all on GitHub. Two public products have been invisible to
topic-based browsing since the day those topics were declared.
What
Applies the enforced signoff default only when a repository is created, so new repos still get it
while existing repos become updatable again. A new check runs in CI so this cannot silently come back.
Restoring the update path also means the config finally acts on what it declares — including two repos
that declare themselves private while being live and public with a deployed site
(
ascoachingogvaner,wedding-app). Both are parked read-only here so that cannot happen as a sideeffect; which side is wrong is your call, tracked on #123.
No repository loses the signoff protection, and deletion stays impossible as before.
Design and diagnosis come from #120; this is that change rebased onto current
main, which it hadconflicted with since #125.
Fixes #112