fix: org-governance release review findings + dev deploy targeting (#727, #733) #741

2026-07-05T19:46:58Z

jmiller commented

2026-07-05 19:46:58 +00:00

Addresses the code-review findings on the org-governance release (#733) plus the root cause of the false-green dev deploy. All must land before #733 goes to main.

Code-review fixes

1. Fail closed on org-rule lookup error (models/git/protected_branch_list.go)
getFirstMatchProtectedBranchRule swallowed errors from FindOrgBranchRuleForBranch (returned nil, nil), so a transient DB error during the org-rule lookup silently dropped the org floor and fell back to the repo rule alone. Now propagates the error → the caller keeps the org rule enforced (pre-receive returns 500, fail-closed).

2. Org rule no longer locks out deploy-key / Actions-bot pushes (models/git/protected_branch_merge.go)
OrgProtectedBranch is team-only, so mergeMostRestrictive was ANDing the repo's WhitelistDeployKeys / WhitelistActionsUser (and the force-push, delete, merge counterparts) against the org side's always-false zero value. Result: the moment any org branch rule matched, every deploy-key and Actions-bot push to that branch was blocked org-wide, with no way for the org (team-only model) to allow them. These seven org-unmanaged fields now carry through from the repo rule unchanged. Team-ID and user-ID intersection (the legitimate floor) is unchanged.

3. Push-policy max-file-size inspects only the delta (routers/private/hook_pre_receive.go)
largestBlobOverLimit ran git ls-tree -r <newTip> — the full tree — so a pre-existing oversized file (committed before the policy existed) permanently blocked all future pushes to the branch. Now diffs oldCommitID..newCommitID (diff-tree + one cat-file --batch-check pass) to inspect only added/modified blobs. New branches (no base commit) still scan the full tree, as every blob is newly introduced. Still fail-open on any error.

Dev deploy targeting fix

deploy-dev.yml drove the dev container image with sed over the shared compose file (/opt/gitea-dev/docker-compose.yml, which defines prod mokogitea, dev mokogitea-dev, runners, mcp). The pattern mokoconsulting/mokogitea:[^ ]* matched the prod service line (the dev service uses a different image name), so every dev deploy:

left the dev service pinned to a stale image → the container recreated but ran old code (this is why the "green" deploys never actually updated dev), and
corrupted the prod image pin in the shared file (latent risk to prod).

Now the dev service image is driven by ${MOKOGITEA_DEV_TAG} (set to the freshly built tag on the deploy command), touching only the dev service — no sed, no prod-line collision.

Companion host change required (not in this PR): the dev service line in /opt/gitea-dev/docker-compose.yml must be switched to image: git.mokoconsulting.tech/mokoconsulting/mokogitea:${MOKOGITEA_DEV_TAG:-…}, and the prod service line reverted to its correct tag. Those are host edits (tracked separately) — this PR only changes the workflow.

Verification (local, Go 1.26.3)

go build ./... → clean (only tests/integration excluded); gofmt clean on all changed files.
deploy-dev.yml: YAML valid, no trailing whitespace.

Related: #733 (release, blocked on this), #738 (tracker), #727.

https://claude.ai/code/session_01Wsno14cxE49MstXFs9G5KT

Addresses the code-review findings on the org-governance release (#733) plus the root cause of the false-green dev deploy. All must land before #733 goes to `main`. ## Code-review fixes **1. Fail closed on org-rule lookup error** (`models/git/protected_branch_list.go`) `getFirstMatchProtectedBranchRule` swallowed errors from `FindOrgBranchRuleForBranch` (returned `nil, nil`), so a transient DB error during the org-rule lookup silently dropped the org floor and fell back to the repo rule alone. Now propagates the error → the caller keeps the org rule enforced (pre-receive returns 500, fail-closed). **2. Org rule no longer locks out deploy-key / Actions-bot pushes** (`models/git/protected_branch_merge.go`) `OrgProtectedBranch` is team-only, so `mergeMostRestrictive` was ANDing the repo's `WhitelistDeployKeys` / `WhitelistActionsUser` (and the force-push, delete, merge counterparts) against the org side's **always-false** zero value. Result: the moment any org branch rule matched, every deploy-key and Actions-bot push to that branch was blocked org-wide, with no way for the org (team-only model) to allow them. These seven org-unmanaged fields now carry through from the repo rule unchanged. Team-ID and user-ID intersection (the legitimate floor) is unchanged. **3. Push-policy max-file-size inspects only the delta** (`routers/private/hook_pre_receive.go`) `largestBlobOverLimit` ran `git ls-tree -r <newTip>` — the **full** tree — so a pre-existing oversized file (committed before the policy existed) permanently blocked all future pushes to the branch. Now diffs `oldCommitID..newCommitID` (`diff-tree` + one `cat-file --batch-check` pass) to inspect only added/modified blobs. New branches (no base commit) still scan the full tree, as every blob is newly introduced. Still fail-open on any error. ## Dev deploy targeting fix **`deploy-dev.yml`** drove the dev container image with `sed` over the **shared** compose file (`/opt/gitea-dev/docker-compose.yml`, which defines prod `mokogitea`, dev `mokogitea-dev`, runners, mcp). The pattern `mokoconsulting/mokogitea:[^ ]*` matched the **prod** service line (the dev service uses a different image name), so every dev deploy: - left the dev service pinned to a stale image → the container recreated but ran **old code** (this is why the "green" deploys never actually updated dev), and - **corrupted the prod image pin** in the shared file (latent risk to prod). Now the dev service image is driven by `${MOKOGITEA_DEV_TAG}` (set to the freshly built tag on the deploy command), touching only the dev service — no `sed`, no prod-line collision. > **Companion host change required (not in this PR):** the dev service line in `/opt/gitea-dev/docker-compose.yml` must be switched to `image: git.mokoconsulting.tech/mokoconsulting/mokogitea:${MOKOGITEA_DEV_TAG:-…}`, and the prod service line reverted to its correct tag. Those are host edits (tracked separately) — this PR only changes the workflow. ## Verification (local, Go 1.26.3) - `go build ./...` → clean (only `tests/integration` excluded); gofmt clean on all changed files. - deploy-dev.yml: YAML valid, no trailing whitespace. Related: #733 (release, blocked on this), #738 (tracker), #727. https://claude.ai/code/session_01Wsno14cxE49MstXFs9G5KT

jmiller added 1 commit 2026-07-05 19:47:00 +00:00

fix: address org-governance release review (#727 , #733 ) + dev deploy targeting

Universal: PR Check / Branch Policy (pull_request) Successful in 1s

Details

PR RC Release / Build RC Release (pull_request) Successful in 3s

Details

Universal: Pre-Release / Build Pre-Release (${{ inputs.stability || github.ref_name }}) (push) Successful in 50s

Details

Universal: PR Check / Validate PR (pull_request) Successful in 11s

Details

Generic: Project CI / Lint & Validate (pull_request) Successful in 35s

Details

Universal: PR Check / Secret Scan (pull_request) Successful in 59s

Details

Branch Cleanup / Delete merged branch (pull_request) Successful in 2s

Details

RC Revert / Rename rc/ back to dev/ (pull_request) Has been skipped

Details

Generic: Project CI / Tests (pull_request) Has been cancelled

Details

Universal: PR Check / Build RC Package (pull_request) Has been cancelled

Details

Universal: PR Check / Report Issues (pull_request) Has been cancelled

Details

e98fca780e

Code-review findings on the org-governance release:

- Fail closed on org-rule lookup error: getFirstMatchProtectedBranchRule
  swallowed FindOrgBranchRuleForBranch errors (returned nil,nil), silently
  dropping the org floor and falling back to the repo rule on a transient DB
  error. Propagate the error so the org rule stays enforced.

- Stop the org rule locking out deploy-key and Actions-bot pushes:
  OrgProtectedBranch is team-only, so mergeMostRestrictive was ANDing the
  repo's WhitelistDeployKeys / WhitelistActionsUser (and the force-push,
  delete and merge counterparts) against the org side's always-false zero
  value, blocking every deploy-key and Actions push in any org with a
  matching branch rule. Carry those org-unmanaged fields through from the
  repo rule unchanged.

- Org push-policy max-file-size now inspects only the pushed delta
  (diff-tree + cat-file --batch-check) instead of the full tip tree via
  ls-tree, so a pre-existing oversized file can no longer permanently block
  unrelated pushes. New branches (no base commit) still scan the full tree.

Dev deploy targeting:

- deploy-dev.yml drove the dev container image via `sed` on the SHARED
  compose file, but the pattern matched the *prod* service line
  (container_name: mokogitea) — leaving the dev service pinned to a stale
  image (so every "green" deploy recreated old code) while corrupting the
  prod image pin. Drive the dev service image from ${MOKOGITEA_DEV_TAG}
  instead; the env-var only affects the dev service.

Claude-Session: https://claude.ai/code/session_01Wsno14cxE49MstXFs9G5KT

jmiller merged commit 7f229ba01c into dev

2026-07-05 19:51:18 +00:00

jmiller deleted branch fix/org-governance-review

2026-07-05 19:51:18 +00:00

jmiller referenced this pull request

2026-07-05 20:44:53 +00:00

Release: org-wide governance series (#727) — dev → main #733

jmiller referenced this pull request

2026-07-05 22:53:41 +00:00

feat: org branch protection per-user (username/email) allowlists + actions-bot toggle (#727) #750

Sign in to join this conversation.