From 18101d253490f0baa1d0ca246e3e176e1dd8660d Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Sat, 30 May 2026 22:16:07 +0530 Subject: [PATCH] fix(api): /claim/preview emits `items` envelope alias for `resources` (B5-P1-3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every list endpoint on the platform (`/api/v1/resources`, `/api/v1/deployments`, `/api/v1/audit`, `/api/v1/backups`, `/api/v1/team/invitations`) wraps results in `items` — but `/claim/preview` shipped with `resources`. Agents generated against the OpenAPI snapshot had to special-case this one envelope shape, and the rule-22 contract surface checklist (CLAUDE.md) flagged it as drift. This PR adds `items` as the canonical alias on both response branches (already_claimed empty + happy-path with rows) while keeping `resources` populated as a deprecated legacy alias — no break for the dashboard, sdk-go, or existing curl recipes. The OpenAPI schema marks `resources` deprecated and routes new clients at `items`. Coverage (rule 17, B5-P1-3 / BugBash 2026-05-20): Symptom: /claim/preview emits `resources`, not `items` Enumeration: grep -n '"resources":\|"items":' internal/handlers/onboarding.go Sites found: 2 (already_claimed branch L114, happy path L178) Sites touched: 2 (both arms emit both keys) Coverage test: TestClaimPreview_ItemsAlias_AlreadyClaimedBranch + TestClaimPreview_ItemsAlias_HappyPathBranch — both assert `items` AND `resources` are present and equal-length on each branch. Live verified: Awaiting deploy + `curl /claim/preview?t= | jq '.items'` Rule 22 surface: - internal/handlers/onboarding.go (the bug) - internal/handlers/openapi.go (ClaimPreviewResponse schema) - openapi.snapshot.json (regenerated via `make openapi-snapshot`) - internal/handlers/claim_preview_items_alias_test.go (coverage gate) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../claim_preview_items_alias_test.go | 142 ++++++++++++++++++ internal/handlers/onboarding.go | 13 +- internal/handlers/openapi.go | 8 +- openapi.snapshot.json | 10 +- 4 files changed, 170 insertions(+), 3 deletions(-) create mode 100644 internal/handlers/claim_preview_items_alias_test.go diff --git a/internal/handlers/claim_preview_items_alias_test.go b/internal/handlers/claim_preview_items_alias_test.go new file mode 100644 index 00000000..4ef84c75 --- /dev/null +++ b/internal/handlers/claim_preview_items_alias_test.go @@ -0,0 +1,142 @@ +package handlers_test + +// claim_preview_items_alias_test.go — coverage for B5-P1-3 (BugBash +// 2026-05-20): GET /claim/preview must emit `items` as the canonical +// envelope field while keeping `resources` populated as the legacy +// back-compat alias. +// +// Two sites in onboarding.go.ClaimPreview emit the response — the +// already_claimed branch and the happy-path branch. The fix must touch +// both. This test exercises both branches and asserts the dual-key +// envelope on each. +// +// The bug was a contract drift: every other list endpoint on the +// platform (/api/v1/resources, /api/v1/deployments, /api/v1/audit, +// /api/v1/backups, /api/v1/team/invitations) uses `items`, but +// /claim/preview shipped with `resources`. Agents generated against +// the OpenAPI snapshot had to special-case this one envelope shape. +// We can't drop `resources` without breaking the live dashboard / +// sdk-go — so this PR adds `items` as the canonical alias and marks +// `resources` deprecated in the spec. + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "instant.dev/internal/crypto" + "instant.dev/internal/testhelpers" +) + +// TestClaimPreview_ItemsAlias_AlreadyClaimedBranch asserts that the +// already_claimed branch of /claim/preview emits BOTH `items` and +// `resources` as empty arrays — the two fields must always co-exist +// so old (dashboard) and new (items-reading) clients both see a +// well-shaped envelope. +func TestClaimPreview_ItemsAlias_AlreadyClaimedBranch(t *testing.T) { + db, clean := testhelpers.SetupTestDB(t) + defer clean() + rdb, cleanRedis := testhelpers.SetupTestRedis(t) + defer cleanRedis() + app := claimPreviewApp(t, db, rdb) + + jti := uuid.New().String() + _, err := db.ExecContext(context.Background(), ` + INSERT INTO onboarding_events (jti, fingerprint, converted_at, team_id) + VALUES ($1, $2, now(), NULL) + `, jti, "fp-items-alias-claimed") + require.NoError(t, err) + + claims := crypto.OnboardingClaims{ + Fingerprint: "fp-items-alias-claimed", + Tokens: []string{}, + RegisteredClaims: jwt.RegisteredClaims{ + ID: jti, + IssuedAt: jwt.NewNumericDate(time.Now()), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), + }, + } + tok := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + signed, err := tok.SignedString([]byte(testhelpers.TestJWTSecret)) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodGet, "/claim/preview?t="+signed, nil) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + var body map[string]any + testhelpers.DecodeJSON(t, resp, &body) + assert.Equal(t, true, body["ok"]) + assert.Equal(t, false, body["token_valid"]) + assert.Equal(t, true, body["already_claimed"]) + + // B5-P1-3: both envelope keys must be present. + items, hasItems := body["items"].([]any) + require.True(t, hasItems, "already_claimed branch must emit `items` envelope key (B5-P1-3)") + assert.Equal(t, 0, len(items), "already_claimed branch: `items` is the empty list") + + resources, hasResources := body["resources"].([]any) + require.True(t, hasResources, "already_claimed branch must also keep emitting legacy `resources` key for back-compat") + assert.Equal(t, 0, len(resources), "already_claimed branch: `resources` legacy alias is the empty list") +} + +// TestClaimPreview_ItemsAlias_HappyPathBranch asserts the dual-key +// envelope on the happy path — the branch that surfaces the actual +// resource list to the caller. Items and resources must be equal-shape +// (both arrays of the same length, same element shape) so a switch +// from one to the other is a true alias swap, not a semantic change. +func TestClaimPreview_ItemsAlias_HappyPathBranch(t *testing.T) { + db, clean := testhelpers.SetupTestDB(t) + defer clean() + rdb, cleanRedis := testhelpers.SetupTestRedis(t) + defer cleanRedis() + provApp, cleanProv := testhelpers.NewTestApp(t, db, rdb) + defer cleanProv() + app := claimPreviewApp(t, db, rdb) + + fp := testhelpers.UniqueFingerprint(t) + res := testhelpers.MustProvisionCacheFull(t, provApp, fp) + require.NotEmpty(t, res.JWT) + + req := httptest.NewRequest(http.MethodGet, "/claim/preview?t="+res.JWT, nil) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + var body map[string]any + testhelpers.DecodeJSON(t, resp, &body) + assert.Equal(t, true, body["ok"]) + assert.Equal(t, true, body["token_valid"]) + + items, hasItems := body["items"].([]any) + require.True(t, hasItems, "happy-path branch must emit `items` envelope key (B5-P1-3)") + assert.GreaterOrEqual(t, len(items), 1, "happy path: `items` must surface at least the provisioned resource") + + resources, hasResources := body["resources"].([]any) + require.True(t, hasResources, "happy-path branch must also keep emitting legacy `resources` key for back-compat") + assert.Equal(t, len(items), len(resources), + "`items` and `resources` must be equal-length aliases — drift means one side is missing rows") + + // Element-shape spot-check: every resource in `items` carries a + // `token` field. (Element-by-element deep-equal between `items` + // and `resources` is enforced by the equal-length + same-source + // invariant; the field-shape check guards against accidental + // projection differences between the two arrays in future edits.) + for i, it := range items { + m, ok := it.(map[string]any) + require.True(t, ok, "items[%d] must be a JSON object", i) + _, hasToken := m["token"] + assert.True(t, hasToken, "items[%d] must carry `token` field (ResourceItem schema)", i) + } + assert.NotEmpty(t, body["expires_at"]) +} diff --git a/internal/handlers/onboarding.go b/internal/handlers/onboarding.go index d3378353..b923073f 100644 --- a/internal/handlers/onboarding.go +++ b/internal/handlers/onboarding.go @@ -107,11 +107,18 @@ func (h *OnboardingHandler) ClaimPreview(c *fiber.Ctx) error { } if ev.ConvertedAt.Valid { + // `items` is the canonical envelope field for every list endpoint on + // the platform (`/api/v1/resources`, `/api/v1/deployments`, audit, + // backups). `/claim/preview` originally shipped with `resources` — + // kept as a legacy alias so the dashboard / sdk-go don't break; new + // callers should read `items`. B5-P1-3 (BugBash 2026-05-20). + empty := []fiber.Map{} return c.JSON(fiber.Map{ "ok": true, "token_valid": false, "already_claimed": true, - "resources": []fiber.Map{}, + "items": empty, + "resources": empty, }) } @@ -172,9 +179,13 @@ func (h *OnboardingHandler) ClaimPreview(c *fiber.Ctx) error { expiresAt = claims.ExpiresAt.Time.UTC().Format(time.RFC3339) } + // Emit BOTH `items` (canonical) and `resources` (legacy alias). Both + // point at the same slice — no allocation overhead. See B5-P1-3 + // note on the already_claimed branch above for rationale. return c.JSON(fiber.Map{ "ok": true, "token_valid": true, + "items": resources, "resources": resources, "expires_at": expiresAt, }) diff --git a/internal/handlers/openapi.go b/internal/handlers/openapi.go index ba18825f..b6a6206d 100644 --- a/internal/handlers/openapi.go +++ b/internal/handlers/openapi.go @@ -2957,9 +2957,15 @@ const openAPISpec = `{ "ok": { "type": "boolean" }, "token_valid": { "type": "boolean", "description": "True when the onboarding JWT is well-formed, unexpired, and not yet claimed." }, "expires_at": { "type": "string", "format": "date-time", "description": "When the onboarding JWT itself expires (typically 7 days from issue). Unrelated to per-resource 24h TTL." }, + "items": { + "type": "array", + "description": "All anonymous resources that this JWT would attach to the new team if /claim were posted. Canonical envelope field — matches /api/v1/resources, /api/v1/deployments, /api/v1/audit, and every other list endpoint on the platform.", + "items": { "$ref": "#/components/schemas/ResourceItem" } + }, "resources": { "type": "array", - "description": "All anonymous resources that this JWT would attach to the new team if /claim were posted.", + "deprecated": true, + "description": "DEPRECATED — legacy alias of items. Kept populated for back-compat with the dashboard and existing curl recipes; new clients should read items. B5-P1-3 (BugBash 2026-05-20).", "items": { "$ref": "#/components/schemas/ResourceItem" } } } diff --git a/openapi.snapshot.json b/openapi.snapshot.json index f76b8cfb..1bd74dee 100644 --- a/openapi.snapshot.json +++ b/openapi.snapshot.json @@ -431,11 +431,19 @@ "format": "date-time", "type": "string" }, + "items": { + "description": "All anonymous resources that this JWT would attach to the new team if /claim were posted. Canonical envelope field — matches /api/v1/resources, /api/v1/deployments, /api/v1/audit, and every other list endpoint on the platform.", + "items": { + "$ref": "#/components/schemas/ResourceItem" + }, + "type": "array" + }, "ok": { "type": "boolean" }, "resources": { - "description": "All anonymous resources that this JWT would attach to the new team if /claim were posted.", + "deprecated": true, + "description": "DEPRECATED — legacy alias of items. Kept populated for back-compat with the dashboard and existing curl recipes; new clients should read items. B5-P1-3 (BugBash 2026-05-20).", "items": { "$ref": "#/components/schemas/ResourceItem" },