feat/subscription-gating

Non-null assertion `org!` relies on opaque helper invariant; prior explicit guard was safer

packages/api/src/middleware/require-active-subscription.ts:37

`if (isOrgAllowed(org)) return ok(undefined); return err(...org!.subscriptionStatus)` — the `!` is sound because `isOrgAllowed(null) === true`, but TypeScript cannot narrow `org` through the boolean return. If `isOrgAllowed` is ever changed to fail-closed on null, this becomes a silent runtime panic. The prior explicit `if (!org) return ok(undefined)` guard made the invariant compiler-checked. Fix: add an explicit null guard before the delegating call, or add a type predicate overload.

`blockedPublicIds` fallback `?? id` would silently expose internal UUID

services/utility/bills/cfe/src/domain/subscription-access.ts:40

`publicIdById.get(id) ?? id` — the fallback is structurally dead (both sets are built in the same loop), but if reached it would log an internal UUID, violating the function's own contract. Replace with a definite assertion (`publicIdById.get(id)!`) or `?? '[unknown]'` to make any future invariant break loud.

`requireActiveSubscription` tests missing `trialing` status

packages/api/src/middleware/__tests__/require-active-subscription.test.ts

The end-to-end path through `requireActiveSubscription → isOrgAllowed → decideFeatureAccess` for a `trialing` org is untested. A regression in the trialing mapping would not be caught.

`decideFeatureAccess` `opts?.exempt` truthy check — safe today, inconsistent with strict policy

domains/core/src/billing/feature-access.decisions.ts:43

All current call paths pass a strict boolean from `isBillingExempt`, so this is harmless. Change to `opts?.exempt === true` for consistency with the documented policy.

`SubscriptionBanner` locale fix is correct — `useLocale()` is the right API

apps/platform/src/components/SubscriptionBanner.tsx:25

Prior `pathname.split('/')[1]` was fragile. `useLocale()` from next-intl is the correct, framework-aware API. Genuine fix.

`partitionGroupsByAccess` UUID matching is correct — no ID-type mismatch

services/utility/bills/cfe/src/domain/subscription-access.ts:39

`group.orgId` and `AccessOrgRow.id` are both internal UUIDs — same type, same source. No mismatch.

Internal UUID still logged on successful job creation (success path not fixed)

services/utility/bills/cfe/src/handlers/subscriber.lambda.ts:348

The refactor switched *blocked* org logging to public IDs via `blockedPublicIds`, but `processRpuGroup`'s success path still emits `orgId: group.orgId` (internal UUID) to CloudWatch. Fix: use `group.orgPublicId` (already present on `RpuGroup`).

`decideFeatureAccess` uses truthy coercion on `opts?.exempt`, not strict `=== true`

domains/core/src/billing/feature-access.decisions.ts:43

`if (opts?.exempt)` is safe today (all callers pass a strict boolean from `isBillingExempt`), but future callers could pass numeric 1 or string 'true' and silently bypass the gate. Change to `opts?.exempt === true` to match `isBillingExempt`'s own strict check.

Non-null `org!` relies on non-local invariant not enforced by the type system

packages/api/src/middleware/require-active-subscription.ts:37

Semantically correct, but non-local reasoning. If `isOrgAllowed` changes null-handling, this becomes unsafe with no compiler warning. An explicit `if (!org || isOrgAllowed(org))` guard would be self-documenting.

`billingExempt` lives in untyped JSONB — no schema enforcement or audit trail

domains/core/src/billing/feature-access.decisions.ts:70

`organizations.metadata` is `Record<string, unknown> | null` with no DB constraint or Zod validation on `billingExempt`. Safe today (no public metadata-write endpoint), but structural: any future metadata patch endpoint opens a bypass. Long-term: migrate to a typed `billing_exempt boolean NOT NULL DEFAULT false` column.

Fail-open on unreadable org is documented and intentional

packages/api/src/middleware/require-active-subscription.ts:35

Policy is sound for the threat model (brief false-allow < locking out paying customer; mrr-sync self-corrects). Edge: a soft-deleted+canceled org silently passes through. Acceptable per stated policy.

`useLocale()` fix eliminates a latent path-manipulation vector in locale extraction

apps/platform/src/components/SubscriptionBanner.tsx:25

Prior `pathname.split('/')[1]` could produce unexpected values on unusual path shapes. `useLocale()` eliminates this entirely.

Exported `isOrgAllowed` helper doesn't signal it requires a DB-fresh org row

domains/core/src/billing/index.ts

The function name doesn't signal that callers must pass a freshly-fetched org. A comment on the export noting this would prevent accidental use with stale/cached org objects.

Non-null assertion `org!` leaks `isOrgAllowed`'s internal invariant into caller

packages/api/src/middleware/require-active-subscription.ts:37

TypeScript still types `org` as `OrgAccessInput | null` after the `isOrgAllowed` check — narrowing is lost because the return type is `boolean`, not a type predicate. The `!` forces readers to reason about another file's implementation. Prefer `if (!org || isOrgAllowed(org)) return ok(undefined)` or a type-predicate overload on `isOrgAllowed`.

Stale JSDoc references `decideFeatureAccess` — impl now calls `isOrgAllowed`

packages/api/src/middleware/require-active-subscription.ts:6

Module-level doc says "applies the pure `decideFeatureAccess` policy" but the implementation now calls `isOrgAllowed` directly. Misleads readers searching for `decideFeatureAccess` usages.

`OrgAccessInput` in decisions file rather than a type file

domains/core/src/billing/feature-access.decisions.ts:73

Canonical form separates types into `{entity}.type.ts`. `OrgAccessInput` is a structural input type — placing it in the decisions file forces consumers to import the decisions module for a type-only import. Minor arch debt; no `feature-access.type.ts` exists yet so this is the pragmatic location.

Missing `PaymentRequiredCode = CodesForStatus<402>` derived type alias

packages/api/src/responses/codes.ts:37

Every other HTTP status group has a `*Code` alias (BadRequestCode, ForbiddenCode, etc.). The new 402 entry has none. Adding `export type PaymentRequiredCode = CodesForStatus<402>` keeps the pattern consistent and enables exhaustive switch typing for 402 mappers.

`subscription-access.ts` not re-exported from domain barrel (intentional but undocumented)

services/utility/bills/cfe/src/domain/subscription-access.ts

The helper is deliberately service-internal but the omission from `domain/index.ts` is a silent pattern deviation. A comment in the barrel noting the deliberate omission would prevent a future contributor from adding it.

`isBillingExempt` numeric-1 truthy case not tested in decisions tests

domains/core/src/billing/__tests__/feature-access.decisions.test.ts:58

The middleware test covers `billingExempt: 1` as non-exempt; the helper's own test covers `'true'` but not `1`. Add `expect(isBillingExempt({ billingExempt: 1 })).toBe(false)` for parity.

`isOrgAllowed` null-metadata test only covers `canceled` — not already-allowed statuses

domains/core/src/billing/__tests__/feature-access.decisions.test.ts:86

A null-metadata case for an allowed status (e.g. `active, metadata: null`) would guard against a future regression where the null-metadata branch accidentally gates allowed statuses.

`requireActiveSubscription` tests missing `trialing` status case

packages/api/src/middleware/__tests__/require-active-subscription.test.ts:14

Tests cover null, active, past_due, canceled, billingExempt, strict-coercion — but not `trialing`. A policy regression mapping `trialing` to `blocked` would go undetected in this suite.

`subscription-access.test.ts` missing deduplication assertion for multi-group canceled org

services/utility/bills/cfe/__tests__/unit/domain/subscription-access.test.ts:14

`partitionGroupsByAccess` deduplicates via `Set<string>`, so a canceled org with 2 groups should appear once in `blockedPublicIds`. This contract is untested — a per-group log refactor would pass all existing tests while silently duplicating log entries.

`partitionGroupsByAccess` empty-groups edge case untested

services/utility/bills/cfe/__tests__/unit/domain/subscription-access.test.ts

`partitionGroupsByAccess([], orgs)` where `orgs` contains blocked orgs: the implementation would populate `blockedPublicIds` even though no groups were dropped. This is a subtle contract question — is 'blocked' about orgs-in-input or orgs-that-had-groups-dropped? The caller's log line would be misleading in the empty-groups case.

Non-null `org!` assertion not type-safe — no type predicate on `isOrgAllowed`

packages/api/src/middleware/require-active-subscription.ts:37

The test correctly verifies the null fail-open path, but there is no type-level guarantee. Making `isOrgAllowed` a type predicate (`org is OrgAccessInput`) or adding an explicit local guard would remove the need for the assertion entirely.

Misleading test name 'exempt is a no-op for an already-allowed status'

domains/core/src/billing/__tests__/feature-access.decisions.test.ts:44

The exempt flag short-circuits the switch, changing `reason` from `'active'` to `'exempt'` — that is not a no-op. Rename to clarify the short-circuit behavior.

`db` mock cast uses `as never` instead of `as unknown as DbOrTx`

packages/api/src/middleware/__tests__/require-active-subscription.test.ts:12

`{} as never` suppresses all type safety. Convention is `{} as unknown as DbOrTx`. Sets a bad precedent even though the mock intercepts before DB is used.

Non-null `org!` can be replaced by a local explicit guard

packages/api/src/middleware/require-active-subscription.ts:37

The cleaner fix is `if (!org || isOrgAllowed(org)) return ok(undefined)` — makes both the null-guard and the policy check visually distinct and assertion-free. Alternatively give `isOrgAllowed` a `(org: null): true` overload so the compiler can narrow.

Dead `?? id` fallback in `partitionGroupsByAccess` should be an assertion

services/utility/bills/cfe/src/domain/subscription-access.ts:40

The fallback is structurally unreachable — both maps are populated in the same loop. The `?? id` fallback obscures this invariant and would silently log an internal UUID if ever reached. Replace with `publicIdById.get(id)!` or `?? '[unknown]'` to make invariant violations loud.

Stale `decideFeatureAccess` reference in module JSDoc

packages/api/src/middleware/require-active-subscription.ts:6

Doc still references `decideFeatureAccess`; impl now delegates to `isOrgAllowed`. Update the module-level comment.

Missing `PaymentRequiredCode` type alias in `codes.ts`

packages/api/src/responses/codes.ts:37

All other HTTP statuses have derived code aliases. A one-liner `export type PaymentRequiredCode = CodesForStatus<402>` restores consistency.

`isOrgAllowed` tests missing `undefined` metadata case

domains/core/src/billing/__tests__/feature-access.decisions.test.ts:86

`OrgAccessInput.metadata` is `... | null` (not `| undefined`), but `isBillingExempt` accepts `undefined`. The two parameter signatures are slightly misaligned. Adding an `undefined` case to `isOrgAllowed` tests would surface the inconsistency.

`describe('SubscriptionInactiveError')` should be `describe('FeatureAccessErrors.subscriptionInactive')`

domains/core/src/billing/__tests__/feature-access.decisions.test.ts:91

The describe block name doesn't match the tested symbol. All other blocks in the file use the function name as the label.

`AccessOrgRow` could extend `OrgAccessInput` to avoid field repetition

services/utility/bills/cfe/src/domain/subscription-access.ts:11

`AccessOrgRow` re-declares `subscriptionStatus` and `metadata` that already exist in `OrgAccessInput`. Using `OrgAccessInput & { id: string; publicId: string }` makes the structural relationship explicit and prevents drift.