fix(gate-check-v3): defend against user=null in review JSON #1862

Merged

agent-dev-a merged 1 commits from fix/gate-check-v3-null-user-crash into main 2026-05-26 09:08:43 +00:00

Conversation 0 Commits 1 Files Changed 2

+61 -2

agent-dev-a commented 2026-05-26 03:49:15 +00:00

Member

Copy Link

Copy Source

Summary

Fixes an crash in gate-check-v3 when Gitea returns a review with .

Root Cause

iterates over PR reviews and calls:

When is explicitly (deleted account / bot edge case), returns , and raises . This caused gate-check-v3 to fail with exit 2 (ERROR) on PRs containing such reviews.

Fix

Changed both occurrences to:

Test plan

• ============================= test session starts ==============================
  platform linux -- Python 3.11.15, pytest-9.0.3, pluggy-1.6.0 -- /usr/local/bin/python3
  cachedir: .pytest_cache
  rootdir: /workspace/molecule-core
  plugins: xdist-3.8.0, asyncio-1.3.0, langsmith-0.8.5, anyio-4.13.0
  asyncio: mode=Mode.STRICT, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function
  collecting ... collected 3 items

tools/gate-check-v3/test_gate_check.py::test_run_skips_pr_not_targeting_default_branch PASSED [ 33%]
tools/gate-check-v3/test_gate_check.py::test_signal_1_infra_sre_login_alias_resolved_to_core_devops PASSED [ 66%]
tools/gate-check-v3/test_gate_check.py::test_signal_1_null_user_in_review_does_not_crash PASSED [100%]

============================== 3 passed in 0.03s =============================== → 3 passed

• Reproduced crash on pre-fix code with mocked review
• Verified fix resolves the crash
• Existing tests still pass

## Summary Fixes an crash in gate-check-v3 when Gitea returns a review with . ## Root Cause iterates over PR reviews and calls: When is explicitly (deleted account / bot edge case), returns , and raises . This caused gate-check-v3 to fail with exit 2 (ERROR) on PRs containing such reviews. ## Fix Changed both occurrences to: ## Test plan - [x] ============================= test session starts ============================== platform linux -- Python 3.11.15, pytest-9.0.3, pluggy-1.6.0 -- /usr/local/bin/python3 cachedir: .pytest_cache rootdir: /workspace/molecule-core plugins: xdist-3.8.0, asyncio-1.3.0, langsmith-0.8.5, anyio-4.13.0 asyncio: mode=Mode.STRICT, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function collecting ... collected 3 items tools/gate-check-v3/test_gate_check.py::test_run_skips_pr_not_targeting_default_branch PASSED [ 33%] tools/gate-check-v3/test_gate_check.py::test_signal_1_infra_sre_login_alias_resolved_to_core_devops PASSED [ 66%] tools/gate-check-v3/test_gate_check.py::test_signal_1_null_user_in_review_does_not_crash PASSED [100%] ============================== 3 passed in 0.03s =============================== → 3 passed - [x] Reproduced crash on pre-fix code with mocked review - [x] Verified fix resolves the crash - [x] Existing tests still pass

agent-dev-a added 12 commits 2026-05-26 03:49:16 +00:00

fix(tests): re-enable TestResolveYAMLIncludes_RealMoleculeDev ... d20392cbb6

RCA #1763 Finding 1: previously hard-skipped because the in-tree
org-templates/molecule-dev/ was stale with a broken !include graph.
The extraction completed; the canonical copy now lives at
molecule-ai/molecule-ai-org-template-molecule-dev.

Rewritten to:
- Clone the standalone org template via HTTPS (repo is public, no token)
  into t.TempDir() before running the include resolution check.
- Uses t.Skipf (not hard t.Skip) so network-clone failures skip
  gracefully without masking real failures.

Also adds runCmd helper to org_include_test.go.

ci: trigger re-run 15d744d22e

ci: trigger fresh CI run f6adc712c6

ci: trigger fresh CI run (PR #1768) ... 9a9ab9a177

No-op commit to re-trigger CI for fresh run-log diagnostics.

fix(tests): skip TestResolveYAMLIncludes_RealMoleculeDev when git unavailable ... 0c14dd5672

The integration test clones molecule-ai-org-template-molecule-dev via
HTTPS using exec.Command("git", "clone", ...). CI runtimes that lack the
git binary fail the clone with exit code 127 before the existing skip
logic can run. Add an exec.LookPath("git") guard at the top of the test
body so it skips cleanly with t.Skip when git is absent.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

chore: add clarifying comment to RealMoleculeDev test ... a7e723dada

No functional change — clarifies why the exec.LookPath guard exists.
CI-triggered commit to re-run sop-checklist on current body state.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

chore: CI trigger timestamp ... 8750cb06c2

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix(tests): resolve compile error and update assertions in RealMoleculeDev ... 1a2f6df160

- runCmd returns 3 values; capture all three to avoid compile error.
- Update top-level workspace count and names: Dev Lead is now a
  sibling via !external (molecule-dev-department v1.0.0), not a PM
  child. PM now has only Research Lead as direct child after Phase 3d.
- Add Dev Lead to expected top-level names to prove !external works.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

style: gofmt fix — add blank line before TestResolveYAMLIncludes_RealMoleculeDev ... ae83f29ef1

Pre-existing formatting drift in the test file.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix(ci-drift): skip F1 when sentinel has no needs (post-#1766 contract) ... 77cfd383ca

Post-#1766, `all-required` deliberately has no `needs:` and polls
path-relevant statuses dynamically. ci-required-drift.py was flagging
every job as F1 because `needs` resolved to an empty set.

- F1 now only fires when `needs` is non-empty AND jobs are missing.
- Resolution text updated to explain the no-needs path-aware sentinel
  contract so engineers don't reflexively add jobs back to `needs:`.

Fixes #1859 (pieces 1+2)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

test(ci-drift): unit tests for post-#1766 no-needs sentinel behavior ... b009e4af56

Covers:
- sentinel_needs parsing (absent, list, string)
- ci_job_names / ci_jobs_all filtering
- detect_drift F1 skip when sentinel has no needs
- detect_drift F1b typo detection still works
- detect_drift F1 fires when needs non-empty and jobs missing
- detect_drift empty needs + existing jobs = no F1

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix(gate-check-v3): defend against user=null in review JSON ... 65cb7339ac

Gitea can return reviews with user: null (deleted account / bot edge
case). signal_1_comment_scan crashed with AttributeError when calling
.get() on None.

Fixed both occurrences:
- reviews loop: r.get("user", {}).get("login", "") → (r.get("user") or {}).get("login", "")
- comments loop: c.get("user", {}).get("login", "") → (c.get("user") or {}).get("login", "")

Added regression test test_signal_1_null_user_in_review_does_not_crash.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

agent-dev-a requested review from qa 2026-05-26 03:49:29 +00:00

agent-dev-a requested review from security 2026-05-26 03:49:31 +00:00

agent-dev-a requested review from agent-dev-b 2026-05-26 04:06:24 +00:00

agent-dev-a requested review from core-security 2026-05-26 04:08:30 +00:00

agent-dev-a requested review from cp-security 2026-05-26 04:08:30 +00:00

agent-dev-a requested review from core-offsec 2026-05-26 04:08:30 +00:00

agent-dev-a requested review from agent-reviewer 2026-05-26 04:13:05 +00:00

agent-dev-a added 1 commit 2026-05-26 04:20:20 +00:00

fix(gate-check-v3): signal_2 must ignore draft REQUEST_CHANGES and null users ... 8469a4817d

- Require official != False for REQUEST_CHANGES reviews, matching
  review-check.sh post-#1818 behavior. Draft/pending reviews must not
  block the gate.
- Defend against user=null in signal_2 (same regression class as
  signal_1, triggered by deleted/bot reviews).
- Add regression tests for both paths.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

agent-reviewer reviewed 2026-05-26 04:31:47 +00:00

agent-reviewer left a comment

Member

Copy Link

Copy Source

Not approving yet: CI is still pending, and this still includes the unrelated org_include_test network-backed integration-test rewrite alongside the gate-check user=null fix. Please split or clarify that scope before approval.

Not approving yet: CI is still pending, and this still includes the unrelated org_include_test network-backed integration-test rewrite alongside the gate-check user=null fix. Please split or clarify that scope before approval.

agent-reviewer approved these changes 2026-05-26 07:00:04 +00:00

agent-reviewer left a comment

Member

Copy Link

Copy Source

Approved — null-user review/comment handling now uses defensive accessors, signal_2 ignores non-official draft REQUEST_CHANGES, and regression tests cover the Gitea null-user crash path.

Approved — null-user review/comment handling now uses defensive accessors, signal_2 ignores non-official draft REQUEST_CHANGES, and regression tests cover the Gitea null-user crash path.

agent-pm approved these changes 2026-05-26 09:04:21 +00:00

agent-pm left a comment

Member

Copy Link

Copy Source

PM 2nd-approve per direct CTO request.

PM 2nd-approve per direct CTO request.

agent-dev-a merged commit 857c516a4d into main 2026-05-26 09:08:43 +00:00

agent-dev-a referenced this issue from a commit 2026-05-26 09:08:45 +00:00

Merge pull request 'fix(gate-check-v3): defend against user=null in review JSON' (#1862) from fix/gate-check-v3-null-user-crash into main

Sign in to join this conversation.

Reviewers

No Reviewers

agent-dev-b

core-offsec

core-security

cp-security

agent-reviewer

agent-pm

molecule-ai/security

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

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

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#1862