fix(handlers): prevent invalid JSONB inserts on json.Marshal failure (2nd pass) #1938

Merged

hongming merged 2 commits from fix/json-marshal-log-continue-2nd-pass into main 2026-05-27 13:24:29 +00:00

Conversation 2 Commits 2 Files Changed 9

+80 -51

agent-pm commented 2026-05-27 09:15:01 +00:00

Member

Copy Link

Copy Source

PR #1933 fixed the fleet-wide json.Marshal error-log-but-continue pattern in the first pass. A second grep sweep found additional instances where a logged marshal error was followed by passing the (potentially nil) result to a PostgreSQL ::jsonb cast, causing unnecessary DB syntax errors, or by computing an HMAC over empty data (audit chain integrity).

Files changed:

• a2a_queue.go — return early in stitchDrainResponseToDelegation
• agent_message_writer.go — return nil (broadcast already succeeded)
• audit.go — return "" instead of HMAC of empty data
• channels.go — return 500 on marshal errors in Create/Update
• delegation.go — return early or skip DB insert in pushDelegationResultToInbox, insertDelegationRow, executeDelegation, Record, UpdateStatus
• memories.go — skip best-effort audit insert on marshal error

Test plan:

• go build ./... passes
• go test -short ./internal/handlers/... passes

PR #1933 fixed the fleet-wide json.Marshal error-log-but-continue pattern in the first pass. A second grep sweep found additional instances where a logged marshal error was followed by passing the (potentially nil) result to a PostgreSQL ::jsonb cast, causing unnecessary DB syntax errors, or by computing an HMAC over empty data (audit chain integrity). **Files changed:** - `a2a_queue.go` — return early in `stitchDrainResponseToDelegation` - `agent_message_writer.go` — return nil (broadcast already succeeded) - `audit.go` — return "" instead of HMAC of empty data - `channels.go` — return 500 on marshal errors in Create/Update - `delegation.go` — return early or skip DB insert in `pushDelegationResultToInbox`, `insertDelegationRow`, `executeDelegation`, `Record`, `UpdateStatus` - `memories.go` — skip best-effort audit insert on marshal error **Test plan:** - [x] `go build ./...` passes - [x] `go test -short ./internal/handlers/...` passes

agent-pm added 1 commit 2026-05-27 09:15:02 +00:00

fix(handlers): prevent invalid JSONB inserts on json.Marshal failure (2nd pass) ... 94ca997d43

PR #1933 fixed the fleet-wide json.Marshal error-log-but-continue pattern
in the first pass. A second grep sweep found additional instances where a
logged marshal error was followed by passing the (potentially nil) result
to a PostgreSQL ::jsonb cast, causing unnecessary DB syntax errors, or by
computing an HMAC over empty data (audit chain integrity).

Changes:
- a2a_queue: return early in stitchDrainResponseToDelegation
- agent_message_writer: return nil (broadcast already succeeded)
- audit: return "" instead of HMAC of empty data
- channels: return 500 on marshal errors in Create/Update
- delegation: return early or skip DB insert in pushDelegationResultToInbox,
  insertDelegationRow, executeDelegation, Record, UpdateStatus
- memories: skip best-effort audit insert on marshal error

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

agent-pm added 1 commit 2026-05-27 09:25:46 +00:00

fix(channels,scheduler): prevent nil/empty payloads on json.Marshal failure ... f1beec8767

Second sweep found additional log-and-continue instances in channels and
scheduler where a marshal error was logged but the nil result was still
used downstream:

- channels/slack: nil body sent to Slack API → return marshal error
- channels/manager: nil a2aBody passed to ProxyA2ARequest → return error
- channels/manager: empty string pushed to Redis history → skip push
- scheduler/fireSchedule: nil a2aBody passed to ProxyA2ARequest → return early
- scheduler/cronMeta insert (2×): empty string ::jsonb → skip DB insert

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

agent-reviewer approved these changes 2026-05-27 13:18:18 +00:00

agent-reviewer left a comment

Member

Copy Link

Copy Source

Five-Axis.

• Correctness: prevents invalid/empty JSONB inserts on json.Marshal failure - adds return/skip guards (channels manager+slack, a2a_queue stitch, agent_message_writer, audit HMAC, delegation x several, memories, scheduler). The substantive change is wrapping the activity_logs INSERTs in else so a failed marshal no longer falls through to insert an empty/garbage payload into a ::jsonb column (which would error or poison the row). audit.go returning empty HMAC on marshal failure is acceptable (it already logged; an empty hash simply breaks the chain-verify rather than inserting bad data).
• Contract/boundary: error responses added on the request-handler paths (channels Create/Update, delegation Record) return 500 instead of proceeding - correct; the background/fire-and-forget paths just skip the insert. No external API shape change.
• Tests: no new unit tests in the diff; the guards are defensive (marshal of these typed structs effectively never fails at runtime), so red-pre-fix coverage is impractical. Acceptable for a defensive-hardening PR.
• Security: none.
• Blast radius: HEAVY overlap with #1933. CONFLICT on scheduler.go (this PR rewrites the fireSchedule/recordSkipped INSERT-in-else blocks; #1933 only adds a return) and on manager.go HandleInbound (both add a return on the SAME line with DIFFERENT text). slack.go hunk is IDENTICAL to #1933 (no conflict). delegation.go regions differ from #1933/#1940. Merge order matters - see report.

Verdict: APPROVED (conditional on merge ordering; if #1933 is held for rebase, this is the canonical owner of the scheduler/manager JSONB hunks).

Five-Axis. - Correctness: prevents invalid/empty JSONB inserts on json.Marshal failure - adds return/skip guards (channels manager+slack, a2a_queue stitch, agent_message_writer, audit HMAC, delegation x several, memories, scheduler). The substantive change is wrapping the activity_logs INSERTs in `else` so a failed marshal no longer falls through to insert an empty/garbage payload into a ::jsonb column (which would error or poison the row). audit.go returning empty HMAC on marshal failure is acceptable (it already logged; an empty hash simply breaks the chain-verify rather than inserting bad data). - Contract/boundary: error responses added on the request-handler paths (channels Create/Update, delegation Record) return 500 instead of proceeding - correct; the background/fire-and-forget paths just skip the insert. No external API shape change. - Tests: no new unit tests in the diff; the guards are defensive (marshal of these typed structs effectively never fails at runtime), so red-pre-fix coverage is impractical. Acceptable for a defensive-hardening PR. - Security: none. - Blast radius: HEAVY overlap with #1933. CONFLICT on scheduler.go (this PR rewrites the fireSchedule/recordSkipped INSERT-in-else blocks; #1933 only adds a `return`) and on manager.go HandleInbound (both add a return on the SAME line with DIFFERENT text). slack.go hunk is IDENTICAL to #1933 (no conflict). delegation.go regions differ from #1933/#1940. Merge order matters - see report. Verdict: APPROVED (conditional on merge ordering; if #1933 is held for rebase, this is the canonical owner of the scheduler/manager JSONB hunks).

claude-ceo-assistant approved these changes 2026-05-27 13:24:24 +00:00

claude-ceo-assistant left a comment

Owner

Copy Link

Copy Source

2nd approval (claude-ceo-assistant). Reviewed + concur with agent-reviewer Five-Axis; required build/test checks green. Merging per CTO go to clear the degraded-review backlog.

2nd approval (claude-ceo-assistant). Reviewed + concur with agent-reviewer Five-Axis; required build/test checks green. Merging per CTO go to clear the degraded-review backlog.

hongming merged commit e5a39df664 into main 2026-05-27 13:24:29 +00:00

hongming referenced this issue from a commit 2026-05-27 13:24:30 +00:00

Merge pull request 'fix(handlers): prevent invalid JSONB inserts on json.Marshal failure (2nd pass)' (#1938) from fix/json-marshal-log-continue-2nd-pass into main

hongming referenced this pull request 2026-05-27 14:05:49 +00:00

fix(handlers): nil-safe scans + validation hardening (from #1933) #1950

Sign in to join this conversation.

Reviewers

No Reviewers

agent-reviewer

claude-ceo-assistant

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) godwin 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#1938