Skip to content

Commit c3a2853

Browse files
committed
fix(stack): close review findings on the combined identity/reorg PR
- integration vitest config: guard against a CS_IT_SUITE glob that resolves to nothing. A renamed/moved integration directory (while the workflow glob still points at the old path) would otherwise drop those suites from CI with the board staying green — the silent-zero-coverage hole the deleted live-coverage-guard used to backstop. Each glob's literal directory prefix must exist; `passWithNoTests: false` covers the fully-empty run. (task #12) - matrix-identity: split the identity-AGNOSTIC `.audit({ metadata })` test into its own describe with a plain (non-federated) client. It was sharing the federation beforeAll, so a Clerk outage / rotated CLERK_MACHINE_TOKEN would fail a test that exercises no identity behaviour. Verified: the audit test passes with no Clerk token while the federated tests require one. - finish the unwrapResult de-dup: lock-context and relational suites now import it from @cipherstash/test-kit instead of each re-declaring a local `unwrap`. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
1 parent cfef834 commit c3a2853

4 files changed

Lines changed: 59 additions & 16 deletions

File tree

packages/stack/integration/drizzle-v3/relational.integration.test.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
eqlTypeSlug as slug,
2929
sortedKeysFor as sortedKeysForKit,
3030
typedEntries,
31+
unwrapResult,
3132
V3_MATRIX,
3233
} from '@cipherstash/test-kit'
3334
import {
@@ -190,11 +191,6 @@ async function selectRowKeys(condition: SQL | undefined): Promise<string[]> {
190191
return rows.map((row) => row.rowKey)
191192
}
192193

193-
function unwrap<T>(result: { data?: T; failure?: { message: string } }): T {
194-
if (result.failure) throw new Error(result.failure.message)
195-
return result.data as T
196-
}
197-
198194
function encryptedInsertRows(): MatrixPlainRow[] {
199195
return ROWS.map((rowKey) => {
200196
const row: MatrixPlainRow = {
@@ -260,7 +256,7 @@ beforeAll(async () => {
260256
)
261257
`)
262258

263-
const encryptedRows = unwrap(
259+
const encryptedRows = unwrapResult(
264260
await client.bulkEncryptModels(encryptedInsertRows(), schema),
265261
)
266262
await db.insert(matrixTable).values(encryptedRows)
@@ -276,7 +272,7 @@ beforeAll(async () => {
276272
// ROW_B exists so the filter proofs below have a row they must EXCLUDE. On a
277273
// one-row table `gt(balance, 0n)` returning every row is indistinguishable
278274
// from it returning the right row.
279-
const bigintRows = unwrap(
275+
const bigintRows = unwrapResult(
280276
await client.bulkEncryptModels(
281277
[
282278
{
@@ -578,7 +574,7 @@ describe('v3 drizzle — relational, needle guards, pagination', () => {
578574
)
579575
expect(encrypted).toHaveLength(1)
580576

581-
const decrypted = unwrap(
577+
const decrypted = unwrapResult(
582578
await client.decryptModel(encrypted[0], bigintSchema),
583579
)
584580
expect(decrypted.balance).toBe(BIGINT_BALANCE)

packages/stack/integration/identity/lock-context.integration.test.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
* control.
3434
*/
3535
import { OidcFederationStrategy } from '@cipherstash/auth'
36-
import { databaseUrl, V3_MATRIX } from '@cipherstash/test-kit'
36+
import { databaseUrl, unwrapResult, V3_MATRIX } from '@cipherstash/test-kit'
3737
import { and, asc as drizzleAsc, eq as drizzleEq, type SQL } from 'drizzle-orm'
3838
import { integer, pgTable, text } from 'drizzle-orm/pg-core'
3939
import { drizzle } from 'drizzle-orm/postgres-js'
@@ -75,11 +75,6 @@ let client: Awaited<ReturnType<typeof EncryptionV3>>
7575
let ops: ReturnType<typeof createEncryptionOperatorsV3>
7676
let db: ReturnType<typeof drizzle>
7777

78-
function unwrap<T>(result: { data?: T; failure?: { message: string } }): T {
79-
if (result.failure) throw new Error(result.failure.message)
80-
return result.data as T
81-
}
82-
8378
/**
8479
* The outcome of a decrypt attempt that is EXPECTED to be denied. `decrypt`
8580
* reports denial as a `Result.failure` rather than throwing, so a bare `await`
@@ -177,7 +172,7 @@ beforeAll(async () => {
177172
`)
178173

179174
// Seed BOTH rows bound to the same lock context.
180-
const encryptedRows = unwrap<Array<Record<string, unknown>>>(
175+
const encryptedRows = unwrapResult<Array<Record<string, unknown>>>(
181176
await client
182177
.bulkEncryptModels(
183178
[
@@ -214,7 +209,7 @@ describe('v3 drizzle operators with lock context (live pg)', () => {
214209
)
215210
expect(row).toBeDefined()
216211

217-
const decrypted = unwrap(
212+
const decrypted = unwrapResult(
218213
await client.decrypt(row.value as never).withLockContext(IDENTITY_CLAIM),
219214
)
220215
expect(decrypted).toBe(SECRET_A)

packages/stack/integration/identity/matrix-identity.integration.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,20 @@ describe('v3 typed client identity-aware operations (live)', () => {
105105
expect(message).toMatch(KEY_DENIAL)
106106
expect(message).not.toMatch(INFRA_FAULT)
107107
}, 30000)
108+
})
109+
110+
// The `.audit({ metadata })` path is identity-AGNOSTIC — it attaches audit
111+
// metadata to an encrypt, with no lock context and no `sub` binding. It needs
112+
// only CS credentials, so it gets its own suite with a plain (non-federated)
113+
// client. Coupling it to the Clerk federation `beforeAll` above would fail this
114+
// test on a Clerk outage or a rotated `CLERK_MACHINE_TOKEN` — for behaviour it
115+
// never exercises.
116+
describe('v3 typed client audit metadata (live)', () => {
117+
let client: Awaited<ReturnType<typeof EncryptionV3<[typeof users]>>>
118+
119+
beforeAll(async () => {
120+
client = await EncryptionV3({ schemas: [users] })
121+
}, 30000)
108122

109123
it('accepts .audit({ metadata }) on the encrypt path and still round-trips', async () => {
110124
const encrypted = unwrapResult(

packages/stack/integration/vitest.config.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { existsSync } from 'node:fs'
12
import { resolve } from 'node:path'
23
import { defineConfig } from 'vitest/config'
34
import { sharedAlias } from '../../../vitest.shared'
@@ -19,13 +20,46 @@ import { sharedAlias } from '../../../vitest.shared'
1920
* Scoping matters: the Drizzle suites talk straight to Postgres and the Supabase
2021
* suites need PostgREST, so a job running both would have to provision both.
2122
*/
23+
const ROOT = resolve(__dirname, '..')
24+
2225
const SUITE_GLOBS = (
2326
process.env['CS_IT_SUITE'] ?? 'integration/**/*.integration.test.ts'
2427
)
2528
.split(',')
2629
.map((glob) => glob.trim())
2730
.filter(Boolean)
2831

32+
/**
33+
* Guard against a glob that resolves to nothing — the silent-zero-coverage hole
34+
* the deleted `live-coverage-guard.test.ts` used to backstop. A directory
35+
* renamed/moved (or a typo) while the workflow's `CS_IT_SUITE` still points at
36+
* the old path makes vitest collect the OTHER globs, pass, and drop those suites
37+
* with no signal — `passWithNoTests` only catches the all-empty case, and the
38+
* no-skips reporter only sees files that were collected.
39+
*
40+
* The literal directory prefix of each glob (everything before the first glob
41+
* metacharacter) must exist. This catches the whole-directory rename directly;
42+
* `passWithNoTests: false` (below) still covers a fully-empty run.
43+
*/
44+
for (const glob of SUITE_GLOBS) {
45+
const literalPrefix = glob
46+
.split('/')
47+
.slice(
48+
0,
49+
glob.split('/').findIndex((seg) => /[*?{}[\]]/.test(seg)),
50+
)
51+
.join('/')
52+
// No metacharacter at all → the glob is a literal file path; check it whole.
53+
const target = /[*?{}[\]]/.test(glob) ? literalPrefix : glob
54+
if (target && !existsSync(resolve(ROOT, target))) {
55+
throw new Error(
56+
`CS_IT_SUITE glob "${glob}" points at "${target}", which does not exist. ` +
57+
'A renamed/moved integration directory would otherwise drop its suites ' +
58+
'from CI silently. Fix the glob (or the directory) so the suites run.',
59+
)
60+
}
61+
}
62+
2963
export default defineConfig({
3064
resolve: {
3165
alias: {
@@ -81,6 +115,10 @@ export default defineConfig({
81115
*/
82116
silent: 'passed-only',
83117

118+
// A fully-empty run is a failure, not a pass: paired with the per-glob
119+
// directory guard above, a mistyped/renamed suite path can never go green.
120+
passWithNoTests: false,
121+
84122
// Fail the run if anything is skipped. A skipped test reads exactly like a
85123
// passing one, and every silent hole this suite has found took that shape.
86124
reporters: ['default', resolve(__dirname, 'no-skips-reporter.ts')],

0 commit comments

Comments
 (0)