fix(core): check RowsAffected errors in admin_schedules_health, org_import, llm_billing_mode #2107

Merged

devops-engineer merged 3 commits from fix/rows-affected-core into main 2026-06-01 23:28:30 +00:00

Conversation 4 Commits 3 Files Changed 6

+34 -7

core-be commented 2026-06-01 19:05:29 +00:00

Member

Copy Link

Copy Source

Three handlers ignored errors from Result.RowsAffected():

• admin_schedules_health.go ReapOrphans: repointedN / disabledN
• org_import.go migrateRuntimeSchedulesFromRemovedPredecessor
• llm_billing_mode.go SetWorkspaceLLMBillingMode (clear + set paths)

All now log/return the error instead of silently discarding it.

• go build ./... clean
• go test ./internal/handlers/... passes

Comprehensive testing performed

• Unit tests in internal/handlers/ pass (go test).
• Verified RowsAffected error paths with existing sqlmock suites.
• No new external dependencies.

Local-postgres E2E run

• N/A: pure handler-layer error-checking change; no schema or DB behavior change.

Staging-smoke verified or pending

• Scheduled post-merge; change is defensive error-checking only.

Root-cause not symptom

Root cause: sql.Result.RowsAffected() errors were silently discarded with _, so a DB driver or connection failure during UPDATE/DELETE would be invisible. Fix: capture and propagate the error.

Five-Axis review walked

• Correctness: error propagation matches existing handler patterns.
• Readability: minimal change, consistent naming.
• Architecture: no structural change.
• Security: no new attack surface.
• Performance: no hot-path impact.

No backwards-compat shim / dead code added

Yes — no shim or dead code.

Memory/saved-feedback consulted

N/A: routine defensive fix following existing patterns.

Three handlers ignored errors from `Result.RowsAffected()`: - `admin_schedules_health.go` `ReapOrphans`: repointedN / disabledN - `org_import.go` `migrateRuntimeSchedulesFromRemovedPredecessor` - `llm_billing_mode.go` `SetWorkspaceLLMBillingMode` (clear + set paths) All now log/return the error instead of silently discarding it. - `go build ./...` clean - `go test ./internal/handlers/...` passes ## Comprehensive testing performed - Unit tests in `internal/handlers/` pass (go test). - Verified RowsAffected error paths with existing sqlmock suites. - No new external dependencies. ## Local-postgres E2E run - N/A: pure handler-layer error-checking change; no schema or DB behavior change. ## Staging-smoke verified or pending - Scheduled post-merge; change is defensive error-checking only. ## Root-cause not symptom Root cause: `sql.Result.RowsAffected()` errors were silently discarded with `_`, so a DB driver or connection failure during UPDATE/DELETE would be invisible. Fix: capture and propagate the error. ## Five-Axis review walked - Correctness: error propagation matches existing handler patterns. - Readability: minimal change, consistent naming. - Architecture: no structural change. - Security: no new attack surface. - Performance: no hot-path impact. ## No backwards-compat shim / dead code added Yes — no shim or dead code. ## Memory/saved-feedback consulted N/A: routine defensive fix following existing patterns.

core-be added 1 commit 2026-06-01 19:05:30 +00:00

fix(core): check RowsAffected errors in admin_schedules_health, org_import, llm_billing_mode ... ee39ccbf2f

Three handlers ignored errors from Result.RowsAffected():

- admin_schedules_health.go: ReapOrphans repointedN / disabledN
- org_import.go: migrateRuntimeSchedulesFromRemovedPredecessor
- llm_billing_mode.go: SetWorkspaceLLMBillingMode (clear + set paths)

All now log/return the error instead of silently discarding it.

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

core-be requested review from core-qa 2026-06-01 19:05:45 +00:00

core-be requested review from core-security 2026-06-01 19:05:47 +00:00

core-be added 1 commit 2026-06-01 19:16:43 +00:00

fix(core): add missing rows.Err() checks in llm_billing_mode and audit ... df6014a34b

readWorkspaceDeriveInputs (llm_billing_mode.go) and scanAuditRows (audit.go)
both iterated rows.Next() without checking rows.Err() after the loop.
Add the check so iteration errors are not silently swallowed.

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

core-be added 1 commit 2026-06-01 20:20:08 +00:00

fix(core): use %w instead of %v for error wrapping in fmt.Errorf ... ef650644cd

Two handlers used %v for error values in fmt.Errorf, preventing
callers from using errors.Is/As. Switch to %w.

- ssrf.go: DNS resolution error
- org_plugin_allowlist.go: requireCallerOwnsOrg error

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

devops-engineer referenced this pull request 2026-06-01 20:22:20 +00:00

fix(handlers): add missing rows.Err() check in llm_billing_mode #2097

core-qa approved these changes 2026-06-01 23:21:15 +00:00

core-qa left a comment

Member

Copy Link

Copy Source

QA approved (#2107). Verified all 6 changes: proper RowsAffected()/rows.Err() error checks (admin_schedules_health, audit, llm_billing_mode, org_import) + %v->%w error wrapping (ssrf, org_plugin_allowlist). Low-risk defensive hardening, idiomatic, matches surrounding error handling. Platform(Go)+all-required green.

QA approved (#2107). Verified all 6 changes: proper RowsAffected()/rows.Err() error checks (admin_schedules_health, audit, llm_billing_mode, org_import) + %v->%w error wrapping (ssrf, org_plugin_allowlist). Low-risk defensive hardening, idiomatic, matches surrounding error handling. Platform(Go)+all-required green.

hongming-ceo-delegated approved these changes 2026-06-01 23:21:15 +00:00

hongming-ceo-delegated left a comment

Member

Copy Link

Copy Source

Approve under CTO authority. Reviewed all 6 rows-err/error-wrap changes; correct + low-risk; CI green.

Approve under CTO authority. Reviewed all 6 rows-err/error-wrap changes; correct + low-risk; CI green.

devops-engineer commented 2026-06-01 23:21:16 +00:00

Member

Copy Link

Copy Source

Non-author SOP ack (devops-engineer, engineers): reviewed the 6-file rows-err/RowsAffected/%w hardening — all correct, low-risk, no logic change. /qa-recheck /security-recheck

Non-author SOP ack (devops-engineer, engineers): reviewed the 6-file rows-err/RowsAffected/%w hardening — all correct, low-risk, no logic change. /qa-recheck /security-recheck

core-security approved these changes 2026-06-01 23:21:16 +00:00

core-security left a comment

Member

Copy Link

Copy Source

Security approved (#2107). Defensive error-handling hardening; ssrf.go change is %v->%w wrapping (no behavior change to the SSRF block). No security regression; improves error propagation. CI green.

Security approved (#2107). Defensive error-handling hardening; ssrf.go change is %v->%w wrapping (no behavior change to the SSRF block). No security regression; improves error propagation. CI green.

devops-engineer closed this pull request 2026-06-01 23:21:43 +00:00

devops-engineer reopened this pull request 2026-06-01 23:21:47 +00:00

devops-engineer merged commit f869da7a93 into main 2026-06-01 23:28:30 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

hongming-ceo-delegated

core-qa

core-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

5 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2107