Skip to content

fix(handlers): unblock Platform (Go) CI — sqlmock budget-check + test loopback - #1725

Merged
HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/platform-go-ci-tests
Apr 23, 2026
Merged

HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/platform-go-ci-tests

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

Reduces handlers-package test failures from 18 → 4 on main by addressing three pre-existing root causes. All 4 prior attempts (#1710, #1718, #1719, #1664) targeted the wrong layer (golangci-lint, which has `continue-on-error: true` and was never blocking).

What was failing

Test pattern Root cause
11× `TestProxyA2A_` + `TestResolveAgentURL_` `httptest.NewServer` URLs hit `isSafeURL` loopback guard, 502 before handler logic runs
Same 11× + 2 more `ProxyA2A` calls `checkWorkspaceBudget` but sqlmock expectations never added
2× `TestAdminMemories_Import_*` 4-arg INSERT vs 5-arg mock mismatch

What this PR changes

1. Test-only SSRF escape hatch (ssrf.go)

  • New `testAllowLoopback` package var — only 127.0.0.0/8 and ::1 relaxed
  • Metadata (169.254), RFC-1918, TEST-NET, CGNAT, link-local stay blocked in every mode
  • Paired `allowLoopbackForTest(t)` helper with auto-cleanup via `t.Cleanup`

2. Budget-check mock helper (`handlers_test.go`)

  • `expectBudgetCheck(mock, workspaceID)` returns empty rows → `checkWorkspaceBudget` fails-open → "no budget limit"
  • Applied to 8 `TestProxyA2A_*` tests in handler-order-correct positions

3. Admin-memories INSERT arg-count fix

  • Removed spurious 5th `sqlmock.AnyArg()` from `_Success` + `_RedactsSecretsBeforeDedup` (handler uses 4-arg INSERT unless payload carries `created_at`)

4. Side fix: bogus example-TLD hostnames

  • `TestResolveAgentURL_CacheHit` + `_CacheMissDBHit` used `cached.example`/`dbhit.example` which fail DNS in `isSafeURL` before the loopback check runs. Swapped to 127.0.0.1 variants; test intent preserved (they never hit the network).

Why the golangci PRs didn't work

`.github/workflows/ci.yml:75-81` runs golangci-lint with `continue-on-error: true`. Disabling errcheck (#1718/#1719) or fixing all lint errors (#1710, 101 files) leaves the real failures — sqlmock + SSRF — untouched.

Remaining failures (out of scope)

Test Root cause Fix later
`TestGitHubToken_NoTokenProvider` handler behavior drift — 500 vs expected 404 #?
`TestWorkspaceList` + `_WithData` Scan expects 20 args, table now has 21 columns schema migration mock update
`TestRegister_ProvisionerURLPreserved` request body shape drift test refresh

Test plan

  • Ran `go test ./internal/handlers/ -count=1` — went from 18 failures to 4 (verified unchanged ones are unrelated)
  • `go build ./...` clean
  • SSRF semantic tests (`TestIsSafeURL`, `TestIsPrivateOrMetadataIP`, `TestIsPrivateOrMetadataIP_SaaSMode`) all still pass
  • CI confirms Platform (Go) goes green (or stays at 4 unrelated failures until their own fix lands)

Supersedes

🤖 Generated with Claude Code

… loopback

Fixes 14 of the 18 failing tests that have been reddening Platform (Go)
CI on main since the 2026-04-18 open-source restructure + 2026-04-21
SSRF-backport. Reduces handlers package failure count 18 → 4
(remaining 4 are unrelated schema/behavior drift — see follow-ups).

Three root causes fixed:

  1. httptest.NewServer binds to 127.0.0.1; isSafeURL rejects loopback.
     Tests that stub workspace URLs via httptest therefore 502'd at
     the SSRF guard before reaching the handler logic they wanted to
     exercise.
     Fix: add `testAllowLoopback` var to ssrf.go + `allowLoopbackForTest(t)`
     helper in handlers_test.go. Only 127.0.0.0/8 and ::1 are relaxed;
     169.254 metadata, RFC-1918, TEST-NET, CGNAT, and link-local
     protections remain active. Flag is paired with t.Cleanup and is
     never touched by production code.

  2. ProxyA2A's checkWorkspaceBudget query (SELECT budget_limit, COALESCE
     (monthly_spend, 0) FROM workspaces WHERE id = $1) was added with the
     restructure but the a2a_proxy_test.go sqlmock expectations never
     caught up, producing "call to Query ... was not expected" on every
     ProxyA2A-exercising test.
     Fix: `expectBudgetCheck(mock, workspaceID)` helper that registers
     an empty-rows expectation (checkWorkspaceBudget fails-open on
     sql.ErrNoRows, so an empty result = "no budget limit"). Added to
     each of the 8 affected TestProxyA2A_* tests in the correct
     position relative to access-control + activity-log expectations.

  3. TestAdminMemories_Import_Success + _RedactsSecretsBeforeDedup
     mocked a 5-arg INSERT when the handler actually issues a 4-arg
     INSERT (workspace_id, content, scope, namespace) unless the
     payload carries a created_at override. Removed the spurious 5th
     AnyArg from both tests; _PreservesCreatedAt is untouched since it
     legitimately uses the 5-arg form.

Also: TestResolveAgentURL_CacheHit and _CacheMissDBHit used bogus
`cached.example` / `dbhit.example` hostnames that fail DNS resolution
inside isSafeURL (which happens BEFORE the loopback check). Swapped to
`127.0.0.1` variants preserving test intent (they never hit the network).

Remaining 4 failures — out of scope for this PR, tracked separately:
  - TestGitHubToken_NoTokenProvider (handler behavior drift — 500 vs 404)
  - TestWorkspaceList + TestWorkspaceList_WithData (Scan arg count —
    workspaces table gained a column, mock not updated)
  - TestRegister_ProvisionerURLPreserved (request body shape drift)

Closes the 4 wrong-target PRs (#1710, #1718, #1719, #1664) that all
tried to silence the symptom by disabling golangci-lint — which has
`continue-on-error: true` in ci.yml and was never the actual blocker.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@HongmingWang-Rabbit
HongmingWang-Rabbit merged commit 64e4c7b into main Apr 23, 2026
9 of 10 checks passed
@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the fix/platform-go-ci-tests branch April 24, 2026 00:11
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
Co-authored-by: agent-dev-a <agent-dev-a@agents.moleculesai.app>
Co-committed-by: agent-dev-a <agent-dev-a@agents.moleculesai.app>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant