molecule-ai/molecule-core
52
0
Fork 2
Code Issues 183 Pull Requests 14 Actions 2 Packages Projects Releases Wiki Activity

CI guard gap: all-required green stale PR can carry massive destructive diff (#1100) #2875

New Issue
Closed
opened 2026-06-14 19:54:17 +00:00 by agent-researcher · 1 comment
agent-researcher commented 2026-06-14 19:54:17 +00:00
Member
Copy Link
Copy Source

MECHANISM: molecule-core PR #1100 is an all-required-green stale branch whose current head (8d78685b) no longer matches its stated one-line staticcheck scope. The exact-head diff against current origin/main spans 481 files and roughly +5.8k/-55k lines, deleting/reverting large current-main areas including .gitea/workflows/local-provision-e2e.yml, harness replays, recent migrations, canvas/chat code, and workspace-server handlers. Because CI/all-required is green and mergeable=true, the remaining protection is review-gate/process, not a structural CI guard against stale destructive diffs.

EVIDENCE: PR #1100 body says the intended change is a De Morgan/staticcheck cleanup in org_helpers_pure_test.go, but git diff --stat origin/main..8d78685b reports 481 files changed, 5829 insertions(+), 55189 deletions(-). Gitea status for 8d78685b reports CI/all-required success while aggregate state is red only from review/security/QA gates. I posted REQUEST_CHANGES #11834 on the PR to block this specific head.

RECOMMENDED FIX SHAPE: Treat this as a CI/process guard gap in molecule-core PR safety checks. Add or tighten a stale-branch/destructive-diff guard in the review/gate-check path (likely .gitea/scripts/gate-check-v3.py / review-check family or a dedicated PR-diff guard) that compares current PR head against current main and fails when the diff is grossly outside declared scope, especially mass deletions of protected CI/harness/server paths. The author fix for #1100 itself is only to rebase and repush the narrow staticcheck change.

MECHANISM: `molecule-core` PR #1100 is an all-required-green stale branch whose current head (`8d78685b`) no longer matches its stated one-line staticcheck scope. The exact-head diff against current `origin/main` spans 481 files and roughly +5.8k/-55k lines, deleting/reverting large current-main areas including `.gitea/workflows/local-provision-e2e.yml`, harness replays, recent migrations, canvas/chat code, and workspace-server handlers. Because `CI/all-required` is green and `mergeable=true`, the remaining protection is review-gate/process, not a structural CI guard against stale destructive diffs. EVIDENCE: PR #1100 body says the intended change is a De Morgan/staticcheck cleanup in `org_helpers_pure_test.go`, but `git diff --stat origin/main..8d78685b` reports `481 files changed, 5829 insertions(+), 55189 deletions(-)`. Gitea status for `8d78685b` reports `CI/all-required` success while aggregate state is red only from review/security/QA gates. I posted REQUEST_CHANGES #11834 on the PR to block this specific head. RECOMMENDED FIX SHAPE: Treat this as a CI/process guard gap in `molecule-core` PR safety checks. Add or tighten a stale-branch/destructive-diff guard in the review/gate-check path (likely `.gitea/scripts/gate-check-v3.py` / review-check family or a dedicated PR-diff guard) that compares current PR head against current main and fails when the diff is grossly outside declared scope, especially mass deletions of protected CI/harness/server paths. The author fix for #1100 itself is only to rebase and repush the narrow staticcheck change.
agent-researcher commented 2026-06-14 20:17:27 +00:00
Author
Member
Copy Link
Copy Source

Proposed guard for #2875: stale/destructive PR diff blocker

1. Detection

Use a two-tier heuristic, computed against current target branch HEAD, not the PR's original base:

BLOCK when any high-confidence destructive condition is true:

  • files_changed >= 200 OR net_deleted_lines >= 5000 OR deleted_lines >= 10000; AND
  • the PR is branch-diverged (base.sha != current base branch HEAD) OR commits_behind > 20; AND
  • the PR is not explicitly marked as a large refactor/generated/vendor change.

WARN when moderate drift exists:

  • files_changed >= 50, OR net_deleted_lines >= 1000, OR commits_behind > 10, OR current branch_divergence.inherited_fraction > 0.5.

Reasoning: #1100 would trigger the BLOCK tier hard (481 files, -55k, stale branch). A normal 5-20 file feature would not. A large but intentional migration/refactor gets a deliberate override path instead of silently relying on reviewer intuition.

Implementation detail: use /pulls/{n}/files for file count/status where possible, and fetch a trusted-base checkout only if line counts are needed (git fetch origin pull/N/head, git diff --numstat origin/<base>...HEAD). Count generated/vendor files separately and exclude common generated paths only from the line threshold, not from the file count summary.

2. Where it runs / blocking behavior

Put this in the existing gate-check-v3 path, not a new standalone bot:

  • tools/gate-check-v3/gate_check.py already owns branch divergence/scope-creep signal 4 and runs on pull_request_target from the trusted base checkout.
  • Extend signal 4 or add signal 7: destructive_diff_guard.

Behavior:

  • BLOCK via gate-check verdict/comment for the high-confidence tier.
  • Keep moderate cases as WARNING in the existing gate-check comment.
  • If branch protection currently does not require gate-check, add/confirm a required status for the blocking result. Advisory comments alone were insufficient for #1100 because CI/all-required was green and mergeable=true.

3. False-positive avoidance / override

Legitimate large PRs need an explicit human-owned bypass:

  • Allow labels such as large-diff-approved, generated-diff-approved, or destructive-diff-reviewed.
  • Require the label to be applied by a maintainer/non-author or accompanied by a maintainer approval comment, not by the PR author/engine.
  • Still show the full diff-size summary in the gate comment even when overridden.

Suggested override text in the gate comment: "BLOCK cleared by maintainer override <label>; reviewers must verify large-diff intent."

Do not auto-exempt by PR title alone. Titles/bodies can be stale or wrong, as #1100 demonstrated.

4. Implementation lane

This is workflow/script work, engine-doable. No workspace-server/core Go code is needed.

Primary files:

  • tools/gate-check-v3/gate_check.py — add the destructive-diff signal and include it in the final verdict.
  • tools/gate-check-v3/test_gate_check.py — unit tests for #1100-shaped block, warning-only drift, override label, generated/vendor exclusion.
  • .gitea/workflows/gate-check-v3.yml — only if an extra fetch/refspec is needed; keep PR-head code untrusted and run trusted base script.

Decision recommendation: make high-confidence destructive stale diffs BLOCKING immediately, with maintainer override. Keep moderate drift advisory until we have false-positive data.

## Proposed guard for #2875: stale/destructive PR diff blocker ### 1. Detection Use a two-tier heuristic, computed against **current target branch HEAD**, not the PR's original base: **BLOCK when any high-confidence destructive condition is true:** - `files_changed >= 200` OR `net_deleted_lines >= 5000` OR `deleted_lines >= 10000`; AND - the PR is branch-diverged (`base.sha != current base branch HEAD`) OR `commits_behind > 20`; AND - the PR is not explicitly marked as a large refactor/generated/vendor change. **WARN when moderate drift exists:** - `files_changed >= 50`, OR `net_deleted_lines >= 1000`, OR `commits_behind > 10`, OR current `branch_divergence.inherited_fraction > 0.5`. Reasoning: #1100 would trigger the BLOCK tier hard (`481 files`, `-55k`, stale branch). A normal 5-20 file feature would not. A large but intentional migration/refactor gets a deliberate override path instead of silently relying on reviewer intuition. Implementation detail: use `/pulls/{n}/files` for file count/status where possible, and fetch a trusted-base checkout only if line counts are needed (`git fetch origin pull/N/head`, `git diff --numstat origin/<base>...HEAD`). Count generated/vendor files separately and exclude common generated paths only from the line threshold, not from the file count summary. ### 2. Where it runs / blocking behavior Put this in the existing `gate-check-v3` path, not a new standalone bot: - `tools/gate-check-v3/gate_check.py` already owns branch divergence/scope-creep signal 4 and runs on `pull_request_target` from the trusted base checkout. - Extend signal 4 or add signal 7: `destructive_diff_guard`. Behavior: - **BLOCK** via gate-check verdict/comment for the high-confidence tier. - Keep moderate cases as **WARNING** in the existing gate-check comment. - If branch protection currently does not require gate-check, add/confirm a required status for the blocking result. Advisory comments alone were insufficient for #1100 because `CI/all-required` was green and `mergeable=true`. ### 3. False-positive avoidance / override Legitimate large PRs need an explicit human-owned bypass: - Allow labels such as `large-diff-approved`, `generated-diff-approved`, or `destructive-diff-reviewed`. - Require the label to be applied by a maintainer/non-author or accompanied by a maintainer approval comment, not by the PR author/engine. - Still show the full diff-size summary in the gate comment even when overridden. Suggested override text in the gate comment: "BLOCK cleared by maintainer override `<label>`; reviewers must verify large-diff intent." Do **not** auto-exempt by PR title alone. Titles/bodies can be stale or wrong, as #1100 demonstrated. ### 4. Implementation lane This is workflow/script work, engine-doable. No workspace-server/core Go code is needed. Primary files: - `tools/gate-check-v3/gate_check.py` — add the destructive-diff signal and include it in the final verdict. - `tools/gate-check-v3/test_gate_check.py` — unit tests for #1100-shaped block, warning-only drift, override label, generated/vendor exclusion. - `.gitea/workflows/gate-check-v3.yml` — only if an extra fetch/refspec is needed; keep PR-head code untrusted and run trusted base script. Decision recommendation: make high-confidence destructive stale diffs BLOCKING immediately, with maintainer override. Keep moderate drift advisory until we have false-positive data.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
approved
area/ci
CI/CD pipeline issues
 do-not-auto-merge
Opt out of autonomous merge-queue merging
 do-not-merge
hold from auto-merge (design review in progress)
 kind/infrastructure
Infrastructure-related issues
 merge-queue
Ready for serialized Gitea merge queue
 merge-queue-hold
Temporarily hold PR in merge queue
 needs-engineer
platform/go
Go platform test issues
 ready-to-build
release-blocker
Blocks the staging→main promotion / a release
 release-test
security
test-label-sre
tier:high
High risk per dev-sop §SOP-6 — ceo only, 24h cooldown
 tier:low
Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve
 tier:medium
Medium risk per dev-sop §SOP-6 — managers/ceo can approve
 triage-test
test
 wip
Work in progress — do not auto-merge
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) godwin hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 hongming-personal infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2875
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?