Skip to content

Commit ba9e026

Browse files
committed
test(satellites): per-satellite merged coverage gate + breaker/isolation hardening
Address four production-readiness concerns from a reviewer (satellite RC honesty, admin auth, circuit breaker vs Redis, concurrency isolation). Three were already sound; the work is tests, CI teeth, and doc reconciliation. Concern 1 - per-satellite MERGED coverage gate. Admin's controllers are integration-tested by design, so a unit-floor bar would be coverage theatre. Instead: - new scripts/check-satellite-coverage.mjs buckets the merged unit+integration lcov by packages/<sat>/src and gates each RC satellite against a declared floor (lasagnaSatellite.minMergedCoverage) - ci.yml uploads each satellite's unit V8 into the aggregate merge and runs the gate (report-only first run; ratchet floors just under the measured numbers, then drop SATELLITE_COV_REPORT_ONLY to enforce) - check-satellite-graduation.mjs now requires the floor declared at the bar - stability.md + upgrade-to-1.0.md stop telling adopters to treat satellite packages as experimental (they are RC; in-core opt-in features stay experimental) Concern 3 - circuit breaker survives a Redis outage. New integration spec forces the wired Redis to reject and proves persist/restore/destroy never throw; unit reinforcement for the no-Redis path; new resilience.md section. Concern 4 - cross-tenant isolation under load. New ~1000-write bounded-concurrency fuzz spec with direct-DB read-back; data-isolation/index.md concurrency section (schema-pg scoped). Concern 2 - admin auth docs point at the demo middleware and the empty-array boot guard. Local gates green (794 unit, build:all, typecheck, lint, knip, docs:build). Integration specs are CI-only.
1 parent 2b9ee37 commit ba9e026

17 files changed

Lines changed: 702 additions & 14 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@adonisjs-lasagna/admin": patch
3+
"@adonisjs-lasagna/sso": patch
4+
"@adonisjs-lasagna/billing": patch
5+
"@adonisjs-lasagna/backup": patch
6+
"@adonisjs-lasagna/websockets": patch
7+
---
8+
9+
Add a declared per-satellite merged-coverage floor to each satellite manifest
10+
(`lasagnaSatellite.minMergedCoverage`) plus a CI gate
11+
(`scripts/check-satellite-coverage.mjs`) that enforces each satellite's MERGED
12+
(unit + integration) source coverage against it. This makes the `release
13+
candidate` label backed by a real per-package number, not just the repo-wide
14+
aggregate, and it respects that controller-heavy satellites (admin) are exercised
15+
by the integration tier rather than by unit tests. The graduation gate now also
16+
requires the floor to be declared at the bar. No public API or runtime behavior
17+
change; the manifest field is internal tooling metadata.

.github/workflows/ci.yml

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,51 @@ jobs:
109109
path: coverage/.v8/unit
110110
if-no-files-found: warn
111111

112+
# Each satellite's UNIT V8 (written by its test:coverage to
113+
# coverage/.v8/<sat>-unit above). The coverage-report job merges these with
114+
# each satellite's integration V8 so check-satellite-coverage.mjs can gate a
115+
# real per-satellite MERGED number. One flat artifact per satellite (like the
116+
# integration uploads) so they extract flat into the shared temp dir.
117+
- name: Upload raw satellite unit coverage (V8) — sso
118+
if: always()
119+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
120+
with:
121+
name: c8-raw-sso-unit
122+
path: coverage/.v8/sso-unit
123+
if-no-files-found: warn
124+
125+
- name: Upload raw satellite unit coverage (V8) — backup
126+
if: always()
127+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
128+
with:
129+
name: c8-raw-backup-unit
130+
path: coverage/.v8/backup-unit
131+
if-no-files-found: warn
132+
133+
- name: Upload raw satellite unit coverage (V8) — admin
134+
if: always()
135+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
136+
with:
137+
name: c8-raw-admin-unit
138+
path: coverage/.v8/admin-unit
139+
if-no-files-found: warn
140+
141+
- name: Upload raw satellite unit coverage (V8) — billing
142+
if: always()
143+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
144+
with:
145+
name: c8-raw-billing-unit
146+
path: coverage/.v8/billing-unit
147+
if-no-files-found: warn
148+
149+
- name: Upload raw satellite unit coverage (V8) — websockets
150+
if: always()
151+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
152+
with:
153+
name: c8-raw-websockets-unit
154+
path: coverage/.v8/websockets-unit
155+
if-no-files-found: warn
156+
112157
# Full report (unused exports / orphaned files / deps) stays informational.
113158
- name: Knip (unused-code report)
114159
run: npm run knip
@@ -791,6 +836,40 @@ jobs:
791836
name: c8-raw-billing-integration
792837
path: coverage/.v8/all
793838

839+
# Satellite UNIT V8, merged into the same temp dir so each satellite's
840+
# MERGED (unit+integration) src coverage is what check-satellite-coverage.mjs
841+
# gates below. websockets has no integration tier here (it runs in the WS e2e
842+
# job without coverage), so its merged number is unit-only (see the gate).
843+
- name: Download raw satellite unit coverage (V8) — sso
844+
uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
845+
with:
846+
name: c8-raw-sso-unit
847+
path: coverage/.v8/all
848+
849+
- name: Download raw satellite unit coverage (V8) — backup
850+
uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
851+
with:
852+
name: c8-raw-backup-unit
853+
path: coverage/.v8/all
854+
855+
- name: Download raw satellite unit coverage (V8) — admin
856+
uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
857+
with:
858+
name: c8-raw-admin-unit
859+
path: coverage/.v8/all
860+
861+
- name: Download raw satellite unit coverage (V8) — billing
862+
uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
863+
with:
864+
name: c8-raw-billing-unit
865+
path: coverage/.v8/all
866+
867+
- name: Download raw satellite unit coverage (V8) — websockets
868+
uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
869+
with:
870+
name: c8-raw-websockets-unit
871+
path: coverage/.v8/all
872+
794873
- name: Aggregate coverage report (unit + integration, remapped to src)
795874
run: npm run coverage:report
796875

@@ -808,12 +887,33 @@ jobs:
808887
# pure-helper extraction added functions the integration tier only
809888
# partially exercises, so its floor stays at 78. Keep a small margin
810889
# under the measured numbers and re-check a real run before ratcheting.
890+
# NOTE: these numbers predate the per-satellite gate work, which also
891+
# merges each satellite's UNIT V8 into this lcov (including websockets
892+
# src, new to the aggregate). The shift is expected to be small and
893+
# net-positive, but re-measure on the next full run before ratcheting.
811894
env:
812895
COV_MIN_LINES: '81' # measured ~82.7%
813896
COV_MIN_FUNCTIONS: '78' # measured ~78.9%
814897
COV_MIN_BRANCHES: '78' # measured ~78.5%
815898
run: node scripts/coverage-gate.mjs coverage/lcov.info
816899

900+
# Per-satellite MERGED coverage gate: buckets coverage/lcov.info by
901+
# packages/<sat>/src and checks each RC satellite against the floor it
902+
# declares in package.json (lasagnaSatellite.minMergedCoverage). This is
903+
# the honest "is this satellite RC-worthy" number, because it counts the
904+
# integration tier, which is where controller-heavy satellites (admin) are
905+
# actually exercised.
906+
#
907+
# FIRST RUN IS REPORT-ONLY (SATELLITE_COV_REPORT_ONLY=1): the floors above
908+
# are conservative placeholders. Read the printed per-satellite numbers from
909+
# this step, ratchet each satellite's minMergedCoverage just under its
910+
# measured actual (lines stays >= 60, the graduation bar), then DELETE the
911+
# env line below to flip this into an enforcing gate.
912+
- name: Per-satellite merged coverage gate
913+
env:
914+
SATELLITE_COV_REPORT_ONLY: '1'
915+
run: node scripts/check-satellite-coverage.mjs coverage/lcov.info
916+
817917
- name: Upload combined coverage
818918
if: always()
819919
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4

docs/docs/data-isolation/index.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,35 @@ flowchart TB
6868
end
6969
```
7070

71+
## Why isolation holds under concurrency
72+
73+
A fair worry with multitenancy is whether two tenants hammering the server at
74+
once can ever cross over. For the default `schema-pg` driver, three properties
75+
make that structurally impossible, not merely unlikely:
76+
77+
- **One pool per tenant, `searchPath` baked into the connection config.** Each
78+
tenant's `tenant_<uuid>` connection is created with its schema already set as
79+
the `searchPath`. The package never runs a shared `SET search_path` on a pooled
80+
connection, so there is no per-query mutation for a concurrent request to race.
81+
- **Pools are keyed by `tenant_<uuid>`.** A query for tenant A can only draw a
82+
connection from tenant A's pool, so a warm pool for B can never serve A's query.
83+
- **`AsyncLocalStorage` keeps `tenancy.currentId()` accurate across `await`.**
84+
Interleaved `await` points and `Promise.all` fan-out do not bleed one request's
85+
tenant context into another.
86+
87+
This is backed by tests, not just argument: a 16-way parallel `tenancy.run()`
88+
scope test proves the context never bleeds, the cross-tenant e2e fires 100
89+
interleaved HTTP writes across 5 tenants with a direct-DB read-back, and a fuzz
90+
spec scales that to roughly 1000 interleaved writes across 10 tenants. Every one
91+
asserts that a tenant's schema holds only its own rows.
92+
93+
This argument is specific to `schema-pg` (and `database-pg`, which goes further
94+
with a whole database per tenant). The `rowscope-pg` driver is different by
95+
design: it shares one connection and scopes every query by a `tenant_id` filter
96+
(with an optional RLS backstop), so its isolation rests on the query-scoping
97+
mixin rather than on per-tenant pools. See
98+
[rowscope-pg](/docs/data-isolation/rowscope-pg).
99+
71100
## Choosing a driver
72101

73102
- **Strict isolation, easy backups, easy per-tenant restore**

docs/docs/resilience.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,42 @@ right default for availability, but it is invisible unless you subscribe to
7777
`DependencyDegraded`. Choose `fail-closed` where correctness beats uptime.
7878
:::
7979

80+
## The tenant circuit breaker
81+
82+
The per-tenant circuit breaker is a separate mechanism from the dependency
83+
policies above, and it answers a common production worry directly: **a Redis
84+
outage cannot take it down.**
85+
86+
The decision is in-memory and per-tenant. Each tenant gets its own in-process
87+
opossum breaker that trips on real `SELECT 1` probes against *that tenant's*
88+
database connection. When a tenant's DB starts failing, its breaker opens and the
89+
tenant fails fast (no more 5-second-timeout probes) while every healthy tenant is
90+
untouched. Redis is **not** in this decision path.
91+
92+
Redis is used for one thing only: a best-effort cache of the OPEN state across
93+
process restarts, so a tenant whose DB was down stays OPEN through a deploy
94+
instead of re-learning it from scratch. Every read and write of that cache is
95+
wrapped in a try/catch that logs a warning and carries on. So when Redis is
96+
unavailable:
97+
98+
- The breaker keeps working entirely from memory. It does not fail open, and it
99+
does not fail closed.
100+
- Persisting a state change (open, close, half-open) logs a warning and moves on.
101+
- Restoring on startup logs a warning and the breaker simply starts CLOSED, then
102+
re-learns the tenant's health from its next probe.
103+
104+
The only degraded case is narrow: if the process restarts *during* a Redis
105+
outage, the persisted OPEN state is lost, so the first request to that tenant
106+
pays one bounded `circuitBreaker.resetTimeout` before the breaker re-trips. That
107+
is the same bounded delay described in
108+
[circuit breaker reopens after a restart](/docs/gotchas#circuit-breaker-reopens-after-a-restart),
109+
and it is deliberate.
110+
111+
The path is covered end to end: a unit test asserts the full open/reset/destroy
112+
cycle never throws with no Redis bound, and an integration test forces the wired
113+
Redis to reject every command and proves the breaker still opens, resets, and
114+
restores without throwing.
115+
80116
## The exception
81117

82118
A `fail-closed` dependency throws `DependencyUnavailableException` instead of a

docs/docs/satellites/admin-rest-api.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,18 @@ curl -H "Authorization: Bearer $TOKEN" \
2828
https://app.example.com/admin/multitenancy/tenants
2929
```
3030

31+
For a concrete starting point, the demo app ships a minimal example at
32+
`examples/api/app/middleware/demo_admin_auth_middleware.ts` (a header check
33+
against an env token). Treat it as an illustration of *where* the guard goes, not
34+
a production auth primitive: swap it for your real session, bearer, or mTLS check.
35+
36+
One footgun the boot guard closes for you: a pattern like
37+
`middleware: authEnabled ? [adminAuth] : []` would, when the flag is off, mount
38+
the destructive routes public while *looking* guarded. The startup check rejects
39+
an empty middleware array the same way it rejects an omitted one, so "looks
40+
guarded but is public" cannot happen. Going public is only possible by writing
41+
`middleware: false` on purpose.
42+
3143
### CSRF
3244

3345
The admin API does **not** apply CSRF protection itself. If you mount it behind a

docs/docs/stability.md

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ The isolation substrate. Everything here is **release candidate** unless noted.
8888
| Tenant resolution (subdomain / path / header) | Release candidate | Always via `resolveTenantId()`. |
8989
| `TenantAdapter` + base-model routing | Release candidate | |
9090
| Connection LRU, budget, optional hard cap | Release candidate | `enforceConnectionCap` defaults `false`; see [scaling limits](/docs/scaling-limits). |
91-
| Circuit breaker | Release candidate | OPEN state restored from Redis across restarts. |
91+
| Circuit breaker | Release candidate | In-memory and per-tenant; survives a Redis outage. See [resilience](/docs/resilience#the-tenant-circuit-breaker). |
9292
| Dependency resilience (`ResilienceService`, 503 fail-closed) | Release candidate | A resolved tenant whose DB is down returns a typed 503, never central. |
9393
| Contextual logging (`AsyncLocalStorage`) | Release candidate | |
9494
| Tenant lifecycle (provision / migrate), hooks, lifecycle events | Release candidate | |
@@ -119,25 +119,40 @@ satellite is published as `>=1.0.0`, so the version string a consumer reads off
119119
npm matches the semver promise this page makes. CI enforces the agreement
120120
mechanically: `scripts/check-stability-versions.mjs` parses this page, and
121121
`scripts/check-satellite-graduation.mjs` verifies each satellite meets the
122-
graduation gate (coverage gate, manifest, configure hook, CHANGELOG, doc page,
123-
version) before it can carry the `release candidate` label.
122+
graduation gate (coverage gate, merged-coverage floor, manifest, configure hook,
123+
CHANGELOG, doc page, version) before it can carry the `release candidate` label.
124124

125125
Each satellite cleared the same graduation bar: an own coverage gate
126-
(`.c8rc.json`) with unit tests over its security-critical core, an
126+
(`.c8rc.json`) with unit tests over its security-critical core, a declared
127+
per-satellite **merged** (unit + integration) coverage floor enforced in CI
128+
(`scripts/check-satellite-coverage.mjs`), so a controller-heavy satellite whose
129+
handlers are exercised by the integration tier is still held to a real number; an
127130
auto-describable `lasagnaSatellite` manifest at the frozen Satellite ABI
128131
(`satelliteApi: 1`), green `publint` + `arethetypeswrong`, a doc page, and a
129132
CHANGELOG. As with the core, `release candidate` (not `stable`) reflects the two
130133
still-open items: an independent security review and production mileage.
131134

135+
**What graduates an in-core opt-in feature to `release candidate`?** The same
136+
bar, scoped to a feature rather than a package: its own coverage at the
137+
graduation floor, a doc page, a stable public surface, and a CHANGELOG entry. The
138+
opt-in features listed above (quotas, webhooks, metrics, audit logs, branding,
139+
feature flags, impersonation) stay `experimental` until they clear it.
140+
132141
## How to read this if you are adopting
133142

134143
- Want true tenant isolation and nothing else? You are on `release-candidate`
135144
ground: the core. Pin the version, follow the [deployment](/docs/deployment)
136145
and [security](/security) guides, and you are leaning only on what is tested
137146
and gated in CI.
138-
- Reaching for a satellite (billing, SSO, admin, backup, quotas, webhooks, and
139-
the rest)? Treat it as `experimental`: it works and is covered by tests, but
140-
pin the version and read the changelog before each upgrade.
147+
- Reaching for a satellite *package* (billing, SSO, admin, backup, websockets)?
148+
You are on `release-candidate` ground too: each cleared the graduation gate
149+
above (frozen Satellite ABI, its own merged coverage floor, doc page, and
150+
CHANGELOG). Pin the version and read the changelog before each upgrade, the
151+
same as the core.
152+
- Using an in-core opt-in feature (quotas, webhooks, metrics, audit logs,
153+
branding, feature flags, impersonation)? Those are still `experimental`: they
154+
work and are covered by tests, but they are not under the 1.x semver promise,
155+
so pin the version and read the changelog before each upgrade.
141156
- Watch this page. As the security review and production mileage close, the core
142157
moves to `stable` and the matrix is updated in the same change.
143158

docs/docs/upgrade-to-1.0.md

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@ the only thing to check is the resolver default at the end.
2626

2727
1.0 narrows its promise on purpose. The isolation **core** is a release
2828
candidate: feature complete and green in CI, with the `stable` label withheld
29-
until an independent security review and production mileage close. The satellites
30-
(billing, SSO, the admin REST API, backup, and the opt-in in-core features like
31-
quotas, webhooks, and metrics) are **experimental** and are not covered by the
32-
1.x semver promise, so they may change in a minor release. The full breakdown and
33-
the per-tier rules are in the [stability matrix](/docs/stability). Pin your
34-
versions accordingly and check the changelog before upgrading an experimental
35-
surface.
29+
until an independent security review and production mileage close. The satellite
30+
**packages** (billing, SSO, the admin REST API, backup, websockets) are release
31+
candidates too: each cleared the same graduation gate as the core (frozen
32+
Satellite ABI, its own merged coverage floor, doc page, CHANGELOG), so they sit
33+
under the 1.x semver promise. The opt-in **in-core features** (quotas, webhooks,
34+
metrics, audit logs, branding, feature flags, impersonation) are still
35+
**experimental** and are not covered by that promise, so they may change in a
36+
minor release. The full breakdown and the per-tier rules are in the
37+
[stability matrix](/docs/stability). Pin your versions accordingly and check the
38+
changelog before upgrading an experimental surface.
3639

3740
## 1. Install the satellites you use
3841

packages/admin/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
"lasagnaSatellite": {
4242
"name": "admin",
4343
"satelliteApi": 1,
44+
"minMergedCoverage": { "lines": 60, "functions": 50, "branches": 50 },
4445
"aliases": ["admin"],
4546
"install": [
4647
"npm install @adonisjs-lasagna/admin"

packages/backup/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
"lasagnaSatellite": {
5454
"name": "backup",
5555
"satelliteApi": 1,
56+
"minMergedCoverage": { "lines": 60, "functions": 50, "branches": 50 },
5657
"aliases": ["backup"],
5758
"provider": "@adonisjs-lasagna/backup/provider",
5859
"commands": "@adonisjs-lasagna/backup/commands",

packages/billing/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
"lasagnaSatellite": {
5757
"name": "billing",
5858
"satelliteApi": 1,
59+
"minMergedCoverage": { "lines": 60, "functions": 50, "branches": 50 },
5960
"aliases": ["billing"],
6061
"migrations": "stubs/migrations",
6162
"requires": ["quotas"],

0 commit comments

Comments
 (0)