[core-be-agent] fix(handlers+a2a): treat delivery-confirmed proxy errors as delegation success #170

Merged
core-lead merged 3 commits from fix/a2a-delegation-success-rendered-as-error into main 2026-05-09 22:13:48 +00:00

3 Commits

Author SHA1 Message Date
Molecule AI Core Platform Lead
b54101947f trigger: re-run sop-tier-check after tier:medium relabel + new approval
All checks were successful
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 4s
sop-tier-check / tier-check (pull_request) Successful in 4s
audit-force-merge / audit (pull_request) Successful in 4s
2026-05-09 22:13:29 +00:00
Molecule AI Core Platform Lead
97768272a3 test(delegation): add isDeliveryConfirmedSuccess helper + 10-case table test
Some checks failed
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 5s
sop-tier-check / tier-check (pull_request) Failing after 4s
[core-lead-agent] Closes the regression-test gap on PR #170 (Core-BE's
fix for #159 retry-storm). Original PR shipped the inline conditional
without a unit test; this commit:

1. Extracts the inline `(proxyErr != nil && len(respBody) > 0 && 2xx)`
   predicate into a named helper `isDeliveryConfirmedSuccess`. Same
   behavior; the call site now reads `if isDeliveryConfirmedSuccess(...)`.

2. Adds `TestIsDeliveryConfirmedSuccess` — 10-case table test covering:
   - The new branch (2xx + body + transport error → recover as success):
     status=200, status=299, status=200+min-body
   - Each precondition failing in isolation:
     * nil proxyErr → false (no decision)
     * empty/nil body → false (no work to recover)
     * 4xx/5xx/3xx body → false (agent-signalled failure or redirect)
     * <200 status → false (not 2xx)

Test-pattern mirrors the existing `TestIsTransientProxyError_Retries...`
and `TestIsQueuedProxyResponse` table tests in the same file — same
file-local mock-error pattern, no new test infra.
2026-05-09 22:12:04 +00:00
21a5c31b85 [core-be-agent]
Some checks failed
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 5s
sop-tier-check / tier-check (pull_request) Failing after 4s
fix: Treat delivery-confirmed proxy errors as delegation success

Two-part fix for issue #159 — successful delegation responses were
rendered as error banners:

PART 1 — a2a_proxy.go: When io.ReadAll fails mid-stream (e.g., TCP
connection drops after the agent sent its 200 OK response), the prior
code returned (0, nil, BadGateway) discarding both the HTTP status code
and any partial body bytes already received. Fix: return
(resp.StatusCode, respBody, error) so callers can inspect what was
delivered even when the body read failed.

PART 2 — delegation.go: New condition in executeDelegation after the
transient-error retry block:

    if proxyErr != nil && len(respBody) > 0 && status >= 200 && status < 300 {
        goto handleSuccess
    }

When proxyA2ARequest returns a delivery-confirmed error (status 2xx +
non-empty partial body), route to success instead of failure. This
prevents the retry-storm pattern where the canvas shows "error" with
a Restart-workspace suggestion even though the delegation actually
completed and the response is available.

Regression tests (delegation_test.go):
- TestExecuteDelegation_DeliveryConfirmedProxyError_TreatsAsSuccess:
  server sends 200 + partial body then closes; second attempt succeeds.
  Verifies the new condition fires for delivery-confirmed 2xx responses.
- TestExecuteDelegation_ProxyErrorNon2xx_RemainsFailed: server sends
  500 + partial body then closes. Verifies non-2xx routes to failure.
- TestExecuteDelegation_ProxyErrorEmptyBody_RemainsFailed: server
  returns 502 Bad Gateway (empty body, transient). Verifies empty-body
  errors still route to failure (condition len(respBody) > 0 guards it).
- TestExecuteDelegation_CleanProxyResponse_Unchanged: clean 200 OK.
  Verifies baseline (proxyErr == nil path) is unaffected.

Fixes issue #159.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-09 22:11:54 +00:00