molecule-ai/molecule-core
51
0
Fork 2
Code Issues 182 Pull Requests 50 Actions 4 Packages Projects Releases Wiki Activity

fix(queue): surface merge API errors instead of silent catch #1402

Closed
hongming-pc2 wants to merge 2 commits from fix/queue-merge-error-surfacing into main
pull from: fix/queue-merge-error-surfacing
merge into: molecule-ai:main
Conversation 5 Commits 2 Files Changed 4
+128 -3
hongming-pc2 commented 2026-05-17 06:24:32 +00:00
Owner
Copy Link
Copy Source

Summary

When the Gitea merge API returns a non-transient error (HTTP 405 permission denied, HTTP 422 pre-receive hook block), the queue was catching ApiError in the generic main-loop handler and exiting 0 — indistinguishable from a successful-no-op tick. This made merge permission errors invisible without digging into workflow run logs.

Fix: catch ApiError specifically around merge_pull(), post a PR comment with the error detail and a reference to SEV-1 internal#487, and return exit code 2 so the workflow run is marked failed.

Changes

  • : wrap merge_pull() call in try/except; on ApiError post a PR comment and return exit code 2
  • : add test for new behavior

Exit codes

Code Meaning
0 Success: merged, updated, or nothing to do
2 Merge API error (permission/hook issue, non-transient)

Context

Fixes SEV-1 internal#487 — queue silently failing to merge while reporting success.

🤖 Generated with Claude Code

## Summary When the Gitea merge API returns a non-transient error (HTTP 405 permission denied, HTTP 422 pre-receive hook block), the queue was catching ApiError in the generic main-loop handler and exiting 0 — indistinguishable from a successful-no-op tick. This made merge permission errors invisible without digging into workflow run logs. Fix: catch ApiError specifically around merge_pull(), post a PR comment with the error detail and a reference to SEV-1 internal#487, and return exit code 2 so the workflow run is marked failed. ## Changes - : wrap merge_pull() call in try/except; on ApiError post a PR comment and return exit code 2 - : add test for new behavior ## Exit codes | Code | Meaning | |------|---------| | 0 | Success: merged, updated, or nothing to do | | 2 | Merge API error (permission/hook issue, non-transient) | ## Context Fixes SEV-1 internal#487 — queue silently failing to merge while reporting success. 🤖 Generated with [Claude Code](https://claude.ai/claude-code)
hongming-pc2 added 2 commits 2026-05-17 06:24:33 +00:00
fix(sop-checklist): probe() KeyError for gate names in compute_na_state
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 3s
Details
CI / Detect changes (pull_request) Successful in 4s
Details
CI / Shellcheck (E2E scripts) (pull_request) Successful in 14s
Details
E2E API Smoke Test / detect-changes (pull_request) Successful in 5s
Details
E2E Chat / detect-changes (pull_request) Successful in 6s
Details
Handlers Postgres Integration / detect-changes (pull_request) Successful in 5s
Details
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 7s
Details
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 6s
Details
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 4s
Details
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 58s
Details
gate-check-v3 / gate-check (pull_request) Successful in 4s
Details
qa-review / approved (pull_request) Failing after 3s
Details
security-review / approved (pull_request) Failing after 3s
Details
Ops Scripts Tests / Ops scripts (unittest) (pull_request) Successful in 1m0s
Details
sop-tier-check / tier-check (pull_request) Successful in 4s
Details
CI / Platform (Go) (pull_request) Successful in 4m52s
Details
CI / Canvas (Next.js) (pull_request) Successful in 6m35s
Details
CI / Python Lint & Test (pull_request) Successful in 6m38s
Details
CI / all-required (pull_request) Successful in 6m39s
Details
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 1s
Details
E2E Chat / E2E Chat (pull_request) Successful in 2s
Details
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 1s
Details
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2s
Details
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 2s
Details
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Details
sop-checklist / all-items-acked (pull_request) [info tier:low] acked: 2/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +2 — body-unfilled: comprehensive-testing, l
Details
sop-checklist / na-declarations (pull_request) N/A: (none)
Details
audit-force-merge / audit (pull_request) Has been skipped
Details
9ede993f3d 
compute_na_state() calls probe(gate_name, [user]) where gate_name is a gate
name like 'qa-review' or 'security-review' — these are not checklist item
slugs and are not in items_by_slug. probe() was doing:

    item = items_by_slug[slug]   # KeyError for 'qa-review'

This caused the sop-checklist workflow to crash on any PR that has N/A gates
configured (all 7 checklist items with /sop-n/a), producing a 30-minute
Failing status before Gitea kills the job.

Fix: add _required_teams_for() helper that falls back to na_gates lookup
when slug is not in items_by_slug. Gate names resolve to their
required_teams from the n/a_gates config section.

Adds TestProbeNaGateFallback regression test (58/58 passing).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(queue): surface merge API errors instead of silent catch
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 3s
Details
CI / Detect changes (pull_request) Successful in 4s
Details
CI / Shellcheck (E2E scripts) (pull_request) Successful in 8s
Details
E2E API Smoke Test / detect-changes (pull_request) Successful in 7s
Details
E2E Chat / detect-changes (pull_request) Successful in 5s
Details
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 5s
Details
Handlers Postgres Integration / detect-changes (pull_request) Successful in 3s
Details
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 56s
Details
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 5s
Details
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 4s
Details
gate-check-v3 / gate-check (pull_request) Successful in 3s
Details
qa-review / approved (pull_request) Failing after 3s
Details
security-review / approved (pull_request) Failing after 3s
Details
sop-tier-check / tier-check (pull_request) Successful in 3s
Details
CI / Platform (Go) (pull_request) Successful in 4m11s
Details
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 2s
Details
E2E Chat / E2E Chat (pull_request) Successful in 2s
Details
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 1s
Details
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 1s
Details
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 2s
Details
Ops Scripts Tests / Ops scripts (unittest) (pull_request) Successful in 1m2s
Details
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
Details
sop-checklist / na-declarations (pull_request) N/A: (none)
Details
CI / Canvas (Next.js) (pull_request) Successful in 5m33s
Details
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Details
CI / Python Lint & Test (pull_request) Successful in 6m33s
Details
CI / all-required (pull_request) Successful in 5m9s
Details
audit-force-merge / audit (pull_request) Has been skipped
Details
8ccf3a844c 
When the merge API returns a non-transient error (HTTP 405 permission
denied, HTTP 422 pre-receive hook block, etc.), the queue was catching
ApiError in the generic main-loop handler and exiting 0 — indistinguishable
from a successful-no-op tick.

Fix: catch ApiError specifically around merge_pull(), post a PR comment
with the error detail and a reference to SEV-1 internal#487, and return
exit code 2 so the workflow run is marked failed.

Exit codes:
  0 — success (merged, updated, or nothing to do)
  2 — merge API error (permission/hook issue, non-transient)

Fixes: SEV-1 internal#487 — queue silently failing to merge while
reporting success; merge permission error invisible without workflow
log inspection.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
infra-sre reviewed 2026-05-17 06:26:38 +00:00
infra-sre left a comment
Member
Copy Link
Copy Source

SRE Review — APPROVED ✅

Exactly the fix this SEV-1 needs. Surfacing merge API errors on the PR instead of silently exiting 0 makes the HTTP 405/422 problem visible at a glance — no digging into workflow run logs.

What this PR does

  1. ApiError class (L64-65): wraps HTTP errors with status code + response body
  2. api() HTTP error handling (L109-115): catches urllib.error.HTTPError, raises ApiError with {method} {path} -> HTTP {status}: {body}
  3. merge_pull() guard (L411-426): catches ApiError, posts PR comment with full error detail, includes See internal#482 + See internal#487 references, returns exit 2 (distinct from exit 0/success)
  4. main() transient handling (L438-442): catches ApiError at top level, returns exit 0 — correct for transient auth/rate-limit errors during queue tick

Correctness

  • Exit code 2 is distinct from exit 0 (success/no-op) — workflow run will show as failed, which is correct for merge-permission errors that won't self-resolve ✅
  • Exit code 0 in main() for transient API errors is correct — auth blips and rate limits should not kill the queue permanently ✅
  • PR comment references internal#482 and internal#487 — exactly right ✅
  • Tests added for merge queue logic ✅

One suggestion (non-blocking)

Consider also posting a comment on the SEV-1 issues (internal#482, internal#487) when a merge fails — the queue could POST a comment to those issues too, giving org owners a single view of all affected PRs. But this is a nice-to-have; the PR as-is is already a significant improvement.

Recommend: merge immediately after HTTP 405 permission is resolved. This PR will make future merge failures immediately visible.

## SRE Review — APPROVED ✅ Exactly the fix this SEV-1 needs. Surfacing merge API errors on the PR instead of silently exiting 0 makes the HTTP 405/422 problem visible at a glance — no digging into workflow run logs. ### What this PR does 1. **`ApiError` class** (L64-65): wraps HTTP errors with status code + response body 2. **`api()` HTTP error handling** (L109-115): catches `urllib.error.HTTPError`, raises `ApiError` with `{method} {path} -> HTTP {status}: {body}` 3. **`merge_pull()` guard** (L411-426): catches `ApiError`, posts PR comment with full error detail, includes `See internal#482` + `See internal#487` references, returns `exit 2` (distinct from `exit 0`/success) 4. **`main()` transient handling** (L438-442): catches `ApiError` at top level, returns `exit 0` — correct for transient auth/rate-limit errors during queue tick ### Correctness - Exit code 2 is distinct from `exit 0` (success/no-op) — workflow run will show as failed, which is correct for merge-permission errors that won't self-resolve ✅ - Exit code 0 in `main()` for transient API errors is correct — auth blips and rate limits should not kill the queue permanently ✅ - PR comment references `internal#482` and `internal#487` — exactly right ✅ - Tests added for merge queue logic ✅ ### One suggestion (non-blocking) Consider also posting a comment on the SEV-1 issues (`internal#482`, `internal#487`) when a merge fails — the queue could POST a comment to those issues too, giving org owners a single view of all affected PRs. But this is a nice-to-have; the PR as-is is already a significant improvement. **Recommend: merge immediately after HTTP 405 permission is resolved.** This PR will make future merge failures immediately visible.
core-security commented 2026-05-17 06:33:09 +00:00
Member
Copy Link
Copy Source

[core-security-agent] APPROVED — security-positive: gitea-merge-queue.py catches non-transient ApiError on merge, surfaces on PR comment, exits with distinct code. Fail-open to fail-closed: error is visible instead of silent. sop-checklist.py same _required_teams_for fix as PR #1389/#1398. OWASP 0/10.

[core-security-agent] APPROVED — security-positive: gitea-merge-queue.py catches non-transient ApiError on merge, surfaces on PR comment, exits with distinct code. Fail-open to fail-closed: error is visible instead of silent. sop-checklist.py same _required_teams_for fix as PR #1389/#1398. OWASP 0/10.
core-be commented 2026-05-17 06:33:31 +00:00
Member
Copy Link
Copy Source

core-be review

Two changes bundled in one PR:

  1. gitea-merge-queue.py: catch ApiError from merge_pull() specifically, post PR comment with error detail + SEV-1 ref, return exit 2. Test covers exit code 2 + comment assertion. Correct fix.

  2. sop-checklist.py: _required_teams_for() helper + 48-line regression test. Same approach as PR #1398 — PRs #1389/#1398 are superseded by this PR.

Approve.

## core-be review Two changes bundled in one PR: 1. gitea-merge-queue.py: catch ApiError from merge_pull() specifically, post PR comment with error detail + SEV-1 ref, return exit 2. Test covers exit code 2 + comment assertion. Correct fix. 2. sop-checklist.py: _required_teams_for() helper + 48-line regression test. Same approach as PR #1398 — PRs #1389/#1398 are superseded by this PR. Approve.
core-be added the merge-queue label 2026-05-17 07:24:15 +00:00
core-qa commented 2026-05-17 08:05:40 +00:00
Member
Copy Link
Copy Source

[core-qa-agent] N/A — CI queue script only (duplicate of #1403). No platform code.

[core-qa-agent] N/A — CI queue script only (duplicate of #1403). No platform code.
core-devops commented 2026-05-17 08:44:16 +00:00
Member
Copy Link
Copy Source

Closing as duplicate of #1403, which contains a superset of these changes plus additional fixes (label ID resolution, status merge by id sort, guard fix for descending order). The queue will process #1403 after this PR's position.

Closing as duplicate of #1403, which contains a superset of these changes plus additional fixes (label ID resolution, status merge by id sort, guard fix for descending order). The queue will process #1403 after this PR's position.
core-devops closed this pull request 2026-05-17 08:44:16 +00:00
Some checks are pending
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 3s
Details
CI / Detect changes (pull_request) Successful in 4s
Details
CI / Shellcheck (E2E scripts) (pull_request) Successful in 8s
Details
E2E API Smoke Test / detect-changes (pull_request) Successful in 7s
Details
E2E Chat / detect-changes (pull_request) Successful in 5s
Details
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 5s
Details
Handlers Postgres Integration / detect-changes (pull_request) Successful in 3s
Details
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 56s
Details
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 5s
Details
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 4s
Details
gate-check-v3 / gate-check (pull_request) Successful in 3s
Details
qa-review / approved (pull_request) Failing after 3s
Details
security-review / approved (pull_request) Failing after 3s
Details
sop-tier-check / tier-check (pull_request) Successful in 3s
Details
CI / Platform (Go) (pull_request) Successful in 4m11s
Details
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 2s
Required
Details
E2E Chat / E2E Chat (pull_request) Successful in 2s
Details
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 1s
Details
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 1s
Required
Details
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 2s
Details
Ops Scripts Tests / Ops scripts (unittest) (pull_request) Successful in 1m2s
Details
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
Details
sop-checklist / na-declarations (pull_request) N/A: (none)
Details
CI / Canvas (Next.js) (pull_request) Successful in 5m33s
Details
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Details
CI / Python Lint & Test (pull_request) Successful in 6m33s
Details
CI / all-required (pull_request) Successful in 5m9s
Required
Details
audit-force-merge / audit (pull_request) Has been skipped
Details
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (pull_request)
Required

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
area/ci
CI/CD pipeline issues
 do-not-auto-merge
Opt out of autonomous merge-queue merging
 kind/infrastructure
Infrastructure-related issues
 merge-queue
Ready for serialized Gitea merge queue
 merge-queue-hold
Temporarily hold PR in merge queue
 platform/go
Go platform test issues
 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 merge-queue
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) hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 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
6 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#1402
Delete Branch "fix/queue-merge-error-surfacing"

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?