fix(repositories): keep org signoff out of updates - #120
Conversation
User evaluationI rendered the active This exercises the render and apply/schema path without changing production. The remaining real-user proof is the provider successfully applying the already-declared topics/settings after the signed artifact deploys, so the PR remains draft until that lifecycle is observed. |
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 (2)
📝 WalkthroughWalkthroughThe shared Repository patch now uses Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🤖 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 @.github/workflows/ci.yaml:
- Around line 38-39: Update the validation block in AGENTS.md to include bash
tests/repository-update-policy.sh alongside the existing pre-PR commands,
keeping the documentation synchronized with the CI step named “Test repository
update policy” and its claim that these commands exactly match CI.
In `@tests/repository-update-policy.sh`:
- Around line 32-43: Update the active Repository validation around the
late_initialized query to require spec.managementPolicies to exactly contain
Observe, Create, and Update, with no missing or additional policies such as
LateInitialize or Delete. Keep archived repositories excluded and fail with the
existing diagnostic path when any active resource violates the exact policy set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cc28522b-15c6-40f0-b53d-a3a54fedf437
📒 Files selected for processing (4)
.github/workflows/ci.yamlAGENTS.mddeploy/repositories/kustomization.yamltests/repository-update-policy.sh
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
.github/workflows/ci.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Use CI to run the required pull-request checks, including the kustomize build and policy/coverage tests.
Files:
.github/workflows/ci.yaml
AGENTS.md
📄 CodeRabbit inference engine (CLAUDE.md)
Follow the instructions defined in
AGENTS.md.Keep this repository's
AGENTS.mdsynchronized with the actualdeploy/layout and shared engineering contract.
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)
The administrative team policy and declarative coverage tests must continue to pass when modifying repository configuration.
Files:
tests/repository-update-policy.sh
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.
Assign ownership and access to themaintainersteam usingTeamandTeamRepositoryresources; do not grant ownership to individual logins.
When adopting an existing resource, setcrossplane.io/external-nameto the live name and use a management policy excludingDelete; after observation, activeRepositoryresources must useObserve/Create/UpdatewithoutLateInitialize.
Verify every Crossplane provider kind and field against the authoritative provider CRDs and generated examples; do not rely on local schema validation.
Before applyingIssueLabelsto a repository with live labels, enumerate existing labels and ensure the declared set is a superset.
Runkubectl kustomize deploy/,bash tests/admin-team-policy.sh, andbash tests/declarative-coverage.shsuccessfully before every pull request.
Files:
deploy/repositories/kustomization.yaml
deploy/repositories/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
deploy/repositories/*.yaml: Place org-owned repository creation defaults such aswebCommitSignoffRequiredininitProvider, so they are applied only during creation.
Define oneRepositoryresource per managed repository, covering settings, merge/signoff, and metadata.
Files:
deploy/repositories/kustomization.yaml
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: devantler-tech/.github
Timestamp: 2026-07-23T05:19:03.739Z
Learning: Use GitHub Issues, specifically the `roadmap`-labelled roadmap structure, as the roadmap of record; implementing pull requests should use `Fixes `#N``.
📚 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
@coderabbitai review |
|
✅ Action performedReview finished.
|
Re-homing this change to Why now. The defect this fixes is live and user-visible. With Why not here. This branch is in the The re-homed PR preserves this design (drop |
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.
Superseded by #127, which carries this design — drop The diagnosis and the approach are this PR's; #127 credits it and adds one thing the rebase exposed: Closing so the branch does not sit conflicted. Nothing here is lost. |
…127) * chore: claim #112 * fix(repositories): keep org-enforced signoff out of update payloads 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. * fix(repositories): park the visibility-drifted repos on Observe 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. * refactor(repositories): drop LateInitialize per repo instead of centrally 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. * docs(repositories): describe the policy each repo actually runs on 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. * fix(repositories): allow Observe-first adoption, and merge initProvider 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.
1 participant
Why
GitHub rejects every declarative Repository update when the provider includes the organization-controlled web commit-signoff field in an unrelated repo PATCH. This leaves repository settings drift unapplied.
What
The security floor stays intact: new repositories still request signoff, Delete remains excluded, and unrelated settings can reconcile without dropping the control.
Validation
Fixes #112