Skip to content

handlers: use plans.Rank + consolidate agent_action constants - #61

Merged
mastermanas805 merged 1 commit into
masterfrom
chore/tier-rank-and-agent-action-cleanup-fresh
May 13, 2026
Merged

mastermanas805 merged 1 commit into
masterfrom
chore/tier-rank-and-agent-action-cleanup-fresh

Conversation

@mastermanas805

Copy link
Copy Markdown
Member

Summary

Two isolated cleanups:

1. Unified TierRank. Delete the two package-private rank functions and route both callers through the shared common/plans.Rank helper introduced in InstaNode-dev/common#10:

  • internal/handlers/billing.go::tierRank — was 6 tiers (anonymous .. team)
  • internal/handlers/admin_customers.go::adminTierRank — was 4 tiers (free .. team) with a different ordering

The discrepancy never bit production because admin never sees anonymous/growth, but the drift was a footgun the moment the admin surface widens. Admin call site now guards against the -1 sentinel explicitly (fromR >= 0 && toR >= 0 && toR < fromR) rather than fromR > 0, because the new canonical mapping places anonymous at rank 0 — a positive-rank check would exclude it as a legitimate fromTier.

2. Consolidate agent_action. Every middleware-level inline string is now a named constant or builder. The contract-review pattern (grep \"agent_action\" internal/middleware) now surfaces one well-named identifier per response shape, not a raw literal:

  • quota.go: extracted 402 prose into quotaExceededAgentAction() (builder, not const, because QuotaUpgradeURL is a var)
  • env_policy.go: extracted 403 fmt.Sprintf into envPolicyDeniedAgentAction()

Both builders document the cross-package mirror with handlers/agent_action.go and the cycle reason for not importing — same pattern as the existing unauthorizedAgentAction (auth.go) and adminForbiddenAgentAction (admin.go). Inline agent_action strings in middleware: 2 → 0.

Depends on

Test plan

  • Build green (go build ./...)
  • go test ./internal/handlers/... -short -count=1 green standalone (incl. TestAgentActionContract, TestAgentActionContract_RegistryCoverage, all env-policy tests, all admin tier tests)
  • go test ./internal/middleware/... -short -count=1 green
  • go test ./internal/plans/... -short -count=1 green
  • No residual references to tierRank / adminTierRank (grep confirms)

Pre-existing flake disclosure: make test-unit shows intermittent failures in instant.dev/internal/handlers (e.g. TestAdminList_AdminUserSees200, TestOnboarding_PostClaim_AlreadyClaimed_Returns409Conflict) that reproduce on master without these changes — they're cross-test DB pollution, not new regressions from this PR. Individual tests + the affected packages run clean standalone.

Generated with Claude Code

Two cleanups, both isolated:

1. Unified TierRank — delete the two package-private rank functions
   (billing.go::tierRank covering 6 tiers, admin_customers.go::adminTierRank
   covering 4 with a different ordering) and route both callers through
   the shared common/plans.Rank helper. Eliminates the drift footgun that
   never bit production only because admin never sees anonymous/growth.

   The admin call site now guards against the -1 sentinel explicitly
   (`fromR >= 0 && toR >= 0 && toR < fromR`) rather than relying on
   `fromR > 0` to exclude unknown tiers — the new canonical mapping
   places anonymous at rank 0, so a positive-rank check would exclude
   it as a legitimate fromTier.

2. Consolidate agent_action — every middleware-level inline string is
   now a named constant or builder, matching the contract-review
   pattern (`grep "agent_action" internal/middleware` surfaces one
   well-named identifier per response shape, not a raw literal):
     - quota.go: extract the 402 prose into quotaExceededAgentAction()
       (builder rather than const because QuotaUpgradeURL is a var so
       tests / self-hosted operators can override it)
     - env_policy.go: extract the 403 fmt.Sprintf into
       envPolicyDeniedAgentAction()

   Both builders document the cross-package mirror with handlers/
   agent_action.go and the cycle reason for not importing — same
   pattern as the existing unauthorizedAgentAction (auth.go) and
   adminForbiddenAgentAction (admin.go). Inline agent_action strings
   in middleware: 2 → 0.

Depends on InstaNode-dev/common#10 (plans.Rank).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mastermanas805
mastermanas805 merged commit 92aa43a into master May 13, 2026
mastermanas805 added a commit that referenced this pull request Jun 5, 2026
…ent integration (docs/ci/01-CI-INTEGRATION-DESIGN.md) (#265)

* test(ci): Wave 3a+4 — multi-tier e2e factory with_resources + Razorpay test-card payment integration

Wave 3a (#60) + Wave 4 (#61) api-side, per docs/ci/01-CI-INTEGRATION-DESIGN.md.
Both are test-infrastructure, token/secret-gated, inert in prod.

Part A — multi-tier test-user factory (extends /internal/e2e/account)
- The tier param + team/growth-400 gate + paid-tier elevation already shipped
  in the prior factory PR; this adds the remaining brief item: with_resources.
- with_resources=true pre-seeds a small set of FAST, row-only resources
  (webhook + cache) on the minted team — synchronous, no backend RPC, tier-
  snapshotted at the team's tier via the same CreateResource→MarkResourceActive
  two-phase lifecycle a real provision uses — so a journey can start populated.
  Seeded tokens surface on the response (seeded_tokens/seeded_count, always a
  non-null array). Rows are reaped with the team by ReapAccount.
- Default tier stays "free" (NOT pro): the brief assumed current default was
  pro, but it has always been free — changing it would silently hand CI a paid
  tier and break the existing empty-body test. Kept free; documented.
- Tests (registry-iterating, rule 18): every allowed tier round-trips +
  reflects in the minted team's plan_tier (anonymous→free team plan);
  every blocked tier (team/growth) → 400 tier_not_allowed; with_resources
  seeds exactly the handler's seed set as active/owned/tier-snapshotted rows;
  omitting it seeds nothing. team-tier-rejected proof retained.

Part B — Razorpay test-card payment integration (Approach B, webhook-injection)
- New billing_testcard_payment_test.go: real-backend, runs in the api test gate
  against test Postgres (NOT the live-k8s e2e suite). Razorpay TEST MODE needs
  no recurring approval, so this is unblocked despite the prod live-checkout
  operator gate.
- Constructs the EXACT subscription.charged body (raw bytes, signed in place,
  created_at inside the ±5-min replay window), signs hex(HMAC-SHA256(rawBody,
  secret)) via the shared signRazorpayPayload (parity with verifyRazorpaySignature),
  POSTs to /razorpay/webhook with X-Razorpay-Signature + x-razorpay-event-id.
- Handler maps subscription→team via resolveTeamFromNotes: notes.team_id (UUID,
  primary) → fallback GetTeamByRazorpaySubscriptionID(sub.id). Test stamps
  notes.team_id.
- Asserts: tier→pro AND an active permanent resource elevated to pro
  (ElevateResourceTiersByTeam, inside UpgradeTeamAllTiersWithSubscription) AND
  the upgrade contract surfaces (plans.Registry resolves pro limits > free for
  the elevated resource — the /api/v1/capabilities source of truth).
- Negatives: tampered body → 400 no upgrade; wrong secret → 400 no upgrade.
- Idempotency: same x-razorpay-event-id twice → deduped:true + exactly one
  razorpay_webhook_events row + tier upgraded once.
- Failure path: payment.failed → 200, NO upgrade (state asserted, not delivery;
  failure email is webhook-gated per project_payment_failure_email_coverage).

Verify: go build/vet ./... green; new factory + payment tests pass on a fresh
test DB. (Local full -p 1 surfaces pre-existing pollution flakes in untouched
packages — models TestLinkGitHubID, storage anon-quota cap — that pass on a
clean DB; CI's ephemeral DB is authoritative.)

Awaiting web wave to drive UI journeys against the multi-tier factory.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(ci): 100% patch coverage for with_resources seed error arms

CI diff-cover flagged 80% patch coverage: the seed-failure branches were
uncovered (internal_e2e_account.go:297-300 seed_failed 503 arm; 374-379
CreateResource/MarkResourceActive error returns).

- Add e2eSeedFastResources package-var seam (mirrors the e2eSignSessionJWT
  seam) so a test can force CreateAccount's seed_failed 503 arm without making
  the real resources table reject an insert mid-request.
- TestE2EAccount_Create_WithResources_SeedFailure_Returns503: seam → 503
  seed_failed.
- internal_e2e_account_seed_whitebox_test.go (sqlmock): the two
  seedFastResources error arms — CreateResource error + MarkResourceActive
  error after a successful insert.

seedFastResources + CreateAccount now report 100.0% func coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(ci): satisfy CreateResource-callsite + error-code registry guards for the seed path

build-and-test surfaced two registry-iterating guard failures from the new
with_resources seed path:

- TestEveryCreateResourceCallSiteIsFollowedByFinalizeProvision: seedFastResources
  calls models.CreateResource without finalizeProvision. That guard targets the
  orphan-generator shape (insert row → backend provision → 201 without atomic
  credential persistence). The seed is CI-only, row-only (webhook/cache), has NO
  backend RPC and NO credential to persist, so the shape can't occur. Added
  internal_e2e_account.go to the allowList with a justifying comment naming the
  alternate path (CreateResource→MarkResourceActive two-phase lifecycle).

- TestErrorCode_HasAgentAction: the new seed_failed code had no codeToAgentAction
  entry. Added it to coverageAllowlist alongside the other CI-only
  /internal/e2e/account codes (machine-to-machine, not customer-facing).

Both guards green locally.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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