Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 38 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,22 @@ jobs:
# column that does not exist on provider_routes shipped and never
# failed a single CI run.
echo "LITELLM_TEST_DB_URL=$dsn" >> "$GITHUB_ENV"
# Same ephemeral database again, read via catalog's own env var
# name (issue #708). TestTenantVisibilityIntegration was gated
# behind CATALOG_TEST_DB_URL, which appeared nowhere under
# .github/, so it never ran; its fixture also hardcoded two
# tenant UUIDs it never inserted into public.tenants, so it would
# have failed its first real run on the FK from
# tenant_model_visibility.tenant_id. Both are fixed by this PR.
echo "CATALOG_TEST_DB_URL=$dsn" >> "$GITHUB_ENV"
# Same ephemeral database again, read via providers' own env var
# name (issue #708). TestProviderCRUDIntegration was gated behind
# PROVIDERS_TEST_DB_URL, which appeared nowhere under .github/, so
# it never ran; its fixture also omitted route_id (a text primary
# key with no default) on its provider_routes insert, so it would
# have failed its first real run on the not-null constraint. Both
# are fixed by this PR.
echo "PROVIDERS_TEST_DB_URL=$dsn" >> "$GITHUB_ENV"

- name: go test — RLS suites against live Postgres (control-plane, edge-api only)
if: >-
Expand All @@ -387,13 +403,29 @@ jobs:
set -euo pipefail
if [ "${{ matrix.module }}" = "control-plane" ]; then
# -tags integration additionally compiles in
# routing/catalog_pricing_integration_test.go (issue #655) and
# litellmconfig/sync_integration_test.go (issue #701), which
# carry a //go:build integration constraint the other packages
# below do not use; passing the tag is a no-op for them.
go test -tags integration -count=1 -v ./internal/accounts/... ./internal/agenttask/... ./internal/egress/... ./internal/payments/... ./internal/profiles/... ./internal/rag/... ./internal/tenants/... ./internal/signup/... ./internal/routing/... ./internal/litellmconfig/...
# routing/catalog_pricing_integration_test.go (issue #655),
# litellmconfig/sync_integration_test.go (issue #701), and
# catalog/catalog_integration_test.go +
# providers/integration_test.go (issue #708), which carry a
# //go:build integration constraint the other packages below do
# not use; passing the tag is a no-op for them.
# -p 1 forces packages to run sequentially rather than in
# parallel across GOMAXPROCS (issue #708 review): several of
# these suites (routing, catalog, providers) share one live
# database and can observe each other's transient rows when run
# concurrently, which is a flaky-collision risk, not a real bug.
go test -tags integration -count=1 -p 1 -v ./internal/accounts/... ./internal/agenttask/... ./internal/egress/... ./internal/payments/... ./internal/profiles/... ./internal/rag/... ./internal/tenants/... ./internal/signup/... ./internal/routing/... ./internal/litellmconfig/... ./internal/catalog/... ./internal/providers/...
else
go test -count=1 -v ./internal/artifacts/... ./internal/rag/...
# -tags integration additionally compiles in
# inference/chat_completions_integration_test.go (issue #708),
# which needs no DB (it mocks LiteLLM and control-plane with
# httptest) and never ran purely because this step never passed
# the tag. ./internal/chat/... and ./internal/auth/... carry
# HIVE_TEST_DB_URL-gated tests that ran neither here nor in the
# plain `go test ./...` step above (that step excludes them via
# -short/skip, this step simply never included the packages);
# both are now wired for real.
go test -tags integration -count=1 -p 1 -v ./internal/artifacts/... ./internal/rag/... ./internal/chat/... ./internal/auth/... ./internal/inference/...
fi

# ------------------------------------------------------------------
Expand Down
1 change: 1 addition & 0 deletions .wolf/buglog.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,4 @@
{"id":"bug-msapxlyj-522e62","timestamp":"2026-08-01T18:42:15.787Z","related_bugs":[],"occurrences":1,"last_seen":"2026-08-01T18:42:15.787Z","error_message":"TypeError: Failed to parse URL from /api/v1/accounts/current/api-keys/{id}/limits, web-console /console/api-keys/{id}/limits returned 500 for every key","root_cause":"getKeyLimits/updateKeyLimits in apps/web-console/lib/api-keys.ts fetched a bare relative path through an injected fetch client. A React Server Component has no origin, so Node fetch rejects the URL before any request is made, and the helpers also carried no session bearer. The page additionally passed a fetch client object down to a Client Component, which is not a serialisable prop.","fix":"Moved the transport into lib/control-plane/client.ts as getApiKeyLimits and updateApiKeyLimits, which use the existing getRequestContext (absolute CONTROL_PLANE_BASE_URL plus session headers). The page reads through the data layer and writes through an inline server action, so no fetch client crosses the Server/Client boundary and the 404 branch keys on ControlPlaneError.status instead of message text.","tags":["web-console","server-component","fetch","issue-552","rsc"]}
{"id":"bug-pr683-authz-limits-read","timestamp":"2026-08-03T15:00:00.000Z","related_bugs":["bug-msapxlyj-522e62"],"occurrences":1,"last_seen":"2026-08-03T15:00:00.000Z","error_message":"an unverified owner holding only api_keys.read got 403 on GET /api/v1/accounts/current/api-keys/{id}/limits and landed in the same React error boundary issue #552/#683 was filed to fix","root_cause":"resolveViewerContext in apps/control-plane/internal/apikeys/http.go gated every api-keys route, including the read-only handleGetKey, handleListKeys and handleGetLimits, on authz.PermAPIKeysWrite, which requires RequiresVerified=true. authz.PermAPIKeysRead requires no verification, so an unverified owner was refused a permission the registry explicitly grants them.","fix":"resolveViewerContext now takes an authz.Permission argument. Read handlers (handleGetKey, handleListKeys, handleGetLimits) pass authz.PermAPIKeysRead; mutating handlers keep authz.PermAPIKeysWrite. Added TestUnverifiedOwnerReadsLimitsButCannotWrite in limits_http_test.go: an unverified owner gets 200 on GET limits and 403 on PUT limits for the same principal.","tags":["control-plane","authz","api-keys","issue-552","issue-683","rsc"]}
{"id": "bug-708-phase19-testmatch-absolute-path", "timestamp": "2026-08-04T00:00:00.000Z", "related_bugs": ["bug-701-litellm-sync-model-id-column"], "occurrences": 1, "last_seen": "2026-08-04T00:00:00.000Z", "error_message": "npx playwright test --project=phase-19 exits 0 and reports only setup tests; all 7 per-push phase-19 spec files (tenant isolation, JWT expiry, cross-tenant attack, audit rows) are silently never collected, no error or warning", "root_cause": "apps/web-console/playwright.config.ts scoped the phase-19 project testMatch to a filename-only regex assuming Playwright tests it against the bare filename. Playwright actually tests testMatch regexes against the full absolute file path (runner/projectUtils.js collectFilesForProject), and an absolute path always contains path separators, so the anchored no-separator-from-position-0 pattern could never match any file. playwright test --list still exits 0 with zero collected specs and no diagnostic, the same silent-skip shape as issue 701.", "fix": "Rewrote testMatch to anchor on the phase-19 directory segment and stop before any further separator, so it matches the 7 direct-child spec files and deliberately excludes the e2e/phase-19/owui subtree, which is a separately configured nightly suite (own baseURL, own auth setup, documented in e2e/phase-19/README.md) that would otherwise get swept in under the wrong dependency chain. Added scripts/verify-phase19-collection.sh plus an npm script, which runs the real collector and fails loud if the project collection count is not exactly 7, verified to fail against the old regex and pass against the fix before commit. Did not wire --project=phase-19 into CI in this change: those specs need live Supabase auth and real upstreams whose pass state is unverified.", "tags": ["web-console", "playwright", "testMatch", "ci-gap", "silent-skip", "issue-708", "issue-701", "phase-19"]}
{"id": "bug-708-dark-go-tests", "date": "2026-08-04", "title": "Five Go DB-backed/integration tests never executed in CI; two had fixture bugs making them impossible to pass on any real schema", "error_message": "catalog: insert or update on table \"tenant_model_visibility\" violates foreign key constraint \"tenant_model_visibility_tenant_id_fkey\" (SQLSTATE 23503). providers: null value in column \"route_id\" of relation \"provider_routes\" violates not-null constraint (SQLSTATE 23502).", "root_cause": "Same failure shape as #701/#705, repeated five times. TestDispatchHappyPathWritesLLMTraceAndAuditsChatRequest (edge-api/internal/chat) and the four TestTenantFallback_* tests (edge-api/internal/auth) had HIVE_TEST_DB_URL correctly wired at the CI job level, but the RLS step's edge-api package list omitted ./internal/chat/... and ./internal/auth/.... TestChatCompletions_ToolCapable_AllowsTools plus four siblings (edge-api/internal/inference) needed only -tags integration, which the edge-api leg never passed. TestTenantVisibilityIntegration (control-plane/internal/catalog) and TestProviderCRUDIntegration (control-plane/internal/providers) were gated behind CATALOG_TEST_DB_URL/PROVIDERS_TEST_DB_URL, neither wired anywhere under .github/, so they never ran; their fixtures also had never been proven against a real schema. The catalog test hardcoded tenant UUIDs a0000000.../b0000000... into tenant_model_visibility without ever inserting them into public.tenants, so the FK on tenant_id was never satisfiable. The providers test's raw provider_routes INSERT omitted route_id, a text primary key with no default, so the not-null constraint was never satisfiable.", "fix": "Fixture fixes only, no assertions weakened: added seedTenant to catalog_integration_test.go inserting tenantA/tenantB into public.tenants (ON CONFLICT DO NOTHING) before any visibility row references them, with cleanup; added an explicit route_id value (route-<slug>) to the provider_routes INSERT in providers/integration_test.go. CI wiring: added CATALOG_TEST_DB_URL and PROVIDERS_TEST_DB_URL to the existing ephemeral-Postgres bootstrap step (same DSN as HIVE_TEST_DB_URL/ROUTING_TEST_DB_URL/LITELLM_TEST_DB_URL); added ./internal/catalog/... and ./internal/providers/... to control-plane's -tags integration invocation; added -tags integration plus ./internal/chat/..., ./internal/auth/..., ./internal/inference/... to edge-api's RLS step. Added a CI=true loud t.Fatal (matching #705's precedent) to every connect/skip helper touched (catalog, providers, chat, auth) so a future missing env var fails the build instead of silently skipping again. apps/edge-api/internal/audio/live_voice_integration_test.go deliberately left untouched: its round trip needs a real GROQ_API_KEY and belongs in the paid live-integration lane, not this free job.", "tags": ["ci-gap", "skipped-test", "integration-test", "fixture-bug", "issue-708", "issue-701", "issue-705", "control-plane", "edge-api", "catalog", "providers", "chat", "auth"]}
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,22 @@ import (
"time"

"github.com/google/uuid"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgxpool"
)

func connectCatalogTestDB(t *testing.T) *pgxpool.Pool {
t.Helper()
dsn := os.Getenv("CATALOG_TEST_DB_URL")
if dsn == "" {
// CI wires CATALOG_TEST_DB_URL at the job level (issue #708); a
// missing value there means this suite silently skipped rather than
// ran, the same failure shape #701/#705 fixed for litellmconfig.
// Fail loud in CI instead of shipping an invisible skip; local dev
// runs (CI unset) still skip.
if os.Getenv("CI") != "" {
t.Fatal("CATALOG_TEST_DB_URL not set in CI; this suite must not silently skip (issue #708)")
}
t.Skip("CATALOG_TEST_DB_URL not set; skipping integration test")
}
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
Expand All @@ -43,6 +52,31 @@ func connectCatalogTestDB(t *testing.T) *pgxpool.Pool {
return pool
}

// seedTenant inserts a test public.tenants row so that tenant_id foreign
// keys on tenant_model_visibility resolve. tenant_model_visibility.tenant_id
// references tenants(id); the test's fixed tenantA/tenantB UUIDs must exist
// as real rows before any visibility row referencing them can be inserted.
// Returns true only if this call created the row (RETURNING with an
// ON CONFLICT DO NOTHING reports no row when one already existed), so the
// caller's cleanup deletes only tenant rows it actually owns.
func seedTenant(t *testing.T, pool *pgxpool.Pool, tenantID uuid.UUID) bool {
t.Helper()
var inserted uuid.UUID
err := pool.QueryRow(context.Background(), `
INSERT INTO public.tenants (id, slug, name, deployment)
VALUES ($1, $2, $2, 'HIVE_CLOUD')
ON CONFLICT (id) DO NOTHING
RETURNING id
`, tenantID, "test-tenant-"+tenantID.String()).Scan(&inserted)
if err != nil {
if err == pgx.ErrNoRows {
return false
}
t.Fatalf("seedTenant %v: %v", tenantID, err)
}
return true
}

// seedAlias inserts a test model_alias row. It is cleaned up via t.Cleanup.
func seedAlias(t *testing.T, pool *pgxpool.Pool, aliasID, visibility string) {
t.Helper()
Expand Down Expand Up @@ -114,6 +148,8 @@ func TestTenantVisibilityIntegration(t *testing.T) {
// Use UUIDs that are very unlikely to collide with real tenant data.
tenantA := uuid.MustParse("a0000000-0000-0000-0000-000000000001")
tenantB := uuid.MustParse("b0000000-0000-0000-0000-000000000001")
ownsTenantA := seedTenant(t, pool, tenantA)
ownsTenantB := seedTenant(t, pool, tenantB)

// Seed two aliases unique to this test run.
suffix := fmt.Sprintf("integ-%d", time.Now().UnixNano())
Expand All @@ -122,12 +158,25 @@ func TestTenantVisibilityIntegration(t *testing.T) {
seedAlias(t, pool, pubAlias, "public")
seedAlias(t, pool, restAlias, "restricted")

// Cleanup visibility rows for both tenants on exit.
// Cleanup visibility rows for both tenants on exit, then the tenant rows
// themselves, but only the ones this test actually created: a tenant row
// already present before this run (ownsTenant* false) is not this test's
// to delete, and might still be in use elsewhere.
t.Cleanup(func() {
deleteVisibilityRow(t, pool, tenantA, pubAlias)
deleteVisibilityRow(t, pool, tenantA, restAlias)
deleteVisibilityRow(t, pool, tenantB, pubAlias)
deleteVisibilityRow(t, pool, tenantB, restAlias)
if ownsTenantA {
if _, err := pool.Exec(context.Background(), "DELETE FROM public.tenants WHERE id = $1", tenantA); err != nil {
t.Errorf("cleanup: delete tenant A: %v", err)
}
}
if ownsTenantB {
if _, err := pool.Exec(context.Background(), "DELETE FROM public.tenants WHERE id = $1", tenantB); err != nil {
t.Errorf("cleanup: delete tenant B: %v", err)
}
}
})

// -------------------------------------------------------------------------
Expand Down
21 changes: 18 additions & 3 deletions apps/control-plane/internal/providers/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ func connectTestDB(t *testing.T) *pgxpool.Pool {
t.Helper()
dsn := os.Getenv("PROVIDERS_TEST_DB_URL")
if dsn == "" {
// CI wires PROVIDERS_TEST_DB_URL at the job level (issue #708); a
// missing value there means this suite silently skipped rather than
// ran, the same failure shape #701/#705 fixed for litellmconfig.
// Fail loud in CI instead of shipping an invisible skip; local dev
// runs (CI unset) still skip.
if os.Getenv("CI") != "" {
t.Fatal("PROVIDERS_TEST_DB_URL not set in CI; this suite must not silently skip (issue #708)")
}
t.Skip("PROVIDERS_TEST_DB_URL not set; skipping integration test")
}
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
Expand Down Expand Up @@ -225,16 +233,23 @@ func TestProviderCRUDIntegration(t *testing.T) {
_, _ = pool.Exec(context.Background(), "DELETE FROM public.model_aliases WHERE alias_id = $1", testAliasID)
})

// route_id is a text primary key with no default; the fixture must supply
// one explicitly. It is derived from slug, which is already unique per
// test run via randomSuffix().
wantRouteID := "route-" + slug
var routeID string
err = pool.QueryRow(ctx, `
INSERT INTO public.provider_routes
(alias_id, provider, provider_model, litellm_model_name, price_class, health_state, priority)
VALUES ($2, $1, 'test-model', 'test/test-model', 'standard', 'healthy', 1)
(route_id, alias_id, provider, provider_model, litellm_model_name, price_class, health_state, priority)
VALUES ($3, $2, $1, 'test-model', 'test/test-model', 'standard', 'healthy', 1)
RETURNING route_id
`, slug, testAliasID).Scan(&routeID)
`, slug, testAliasID, wantRouteID).Scan(&routeID)
if err != nil {
t.Fatalf("step 6: INSERT provider_routes with custom slug: %v", err)
}
if routeID != wantRouteID {
t.Fatalf("step 6: expected route_id %q, got %q", wantRouteID, routeID)
}
// Cleanup route row before alias row (FK order).
t.Cleanup(func() {
_, _ = pool.Exec(context.Background(), "DELETE FROM public.provider_routes WHERE route_id = $1", routeID)
Expand Down
16 changes: 13 additions & 3 deletions apps/edge-api/internal/auth/tenant_fallback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,23 @@ import (
)

// newPool mirrors internal/chat/dispatch_test.go's helper of the same
// name: it skips the test rather than failing when no test DB is wired
// up, so these DB-backed branches only run where HIVE_TEST_DB_URL is set
// (CI integration job / local docker-compose test profile).
// name: locally it skips the test when no test DB is wired up, but in CI
// (CI=true) a missing HIVE_TEST_DB_URL fails loudly instead, so these
// DB-backed branches only run where HIVE_TEST_DB_URL is set (CI integration
// job / local docker-compose test profile).
func newPool(t *testing.T, ctx context.Context) *pgxpool.Pool {
t.Helper()
dsn := os.Getenv("HIVE_TEST_DB_URL")
if dsn == "" {
// CI wires HIVE_TEST_DB_URL at the job level (issue #708), but only
// for the RLS step, which does not pass -short. The plain `go test
// ./... -short` step compiles this package too and runs first,
// before that DSN ever exists, so testing.Short() is required here
// to tell "the RLS step forgot the DSN" (real bug) apart from "this
// is the earlier -short step, which never has it" (expected).
if os.Getenv("CI") != "" && !testing.Short() {
t.Fatal("HIVE_TEST_DB_URL not set in CI; this suite must not silently skip (issue #708)")
}
t.Skip("HIVE_TEST_DB_URL not set")
}
pool, err := pgxpool.New(ctx, dsn)
Expand Down
9 changes: 9 additions & 0 deletions apps/edge-api/internal/chat/dispatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,15 @@ func newPool(t *testing.T, ctx context.Context) *pgxpool.Pool {
t.Helper()
dsn := os.Getenv("HIVE_TEST_DB_URL")
if dsn == "" {
// CI wires HIVE_TEST_DB_URL at the job level (issue #708), but only
// for the RLS step, which does not pass -short. The plain `go test
// ./... -short` step compiles this package too and runs first,
// before that DSN ever exists, so testing.Short() is required here
// to tell "the RLS step forgot the DSN" (real bug) apart from "this
// is the earlier -short step, which never has it" (expected).
if os.Getenv("CI") != "" && !testing.Short() {
t.Fatal("HIVE_TEST_DB_URL not set in CI; this suite must not silently skip (issue #708)")
}
t.Skip("HIVE_TEST_DB_URL not set")
}
pool, err := pgxpool.New(ctx, dsn)
Expand Down
Loading