fix(integration): webhook v2 events, storage migrate, test theatre

- F1: DOCUMENT_DECLINED handler (v2 Decline vs Reject) — routes to same
  handler as DOCUMENT_REJECTED until product refines downstream UX
- Add RECIPIENT_VIEWED / RECIPIENT_SIGNED v2-alias cases with telemetry
  logging so we see when v2 deployments emit them
- D1: populate TABLES_WITH_STORAGE_KEYS (files, berth_pdf_versions,
  brochure_versions, gdpr_exports) — was an empty list, migrated 0 files
- MinIO putObject/getObject/statObject/removeObject socket timeout wrapper
  to prevent worker hangs on TCP blackhole (30s deadline)
- E1: convert test.skip on smoke-setup infra failure to throw new Error
  so green-skipped silence becomes a real test failure (Playwright
  doesn't expose vitest's expect.fail)
- Regression tests: folderId='' → null transform, applyEntityRestoredSuffix
  no-op (never-archived), syncEntityFolderName collision loop past (2)

Note: matching .env.example documentation (D2 — bare DOCUMENSO_API_URL,
DOCUMENSO_API_VERSION, MINIO_AUTO_CREATE_BUCKET, DOCUMENSO_TEMPLATE_ID_EOI,
recipient role id vars) prepared but not committed — pre-commit hook
blocks .env*. Apply manually via the separate .env workflow.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-11 14:02:26 +02:00
parent 955911302b
commit 9a5ba87d6c
7 changed files with 352 additions and 38 deletions

View File

@@ -145,9 +145,19 @@ export async function POST(req: NextRequest): Promise<NextResponse> {
try {
switch (event) {
case 'DOCUMENT_SIGNED':
case 'DOCUMENT_RECIPIENT_COMPLETED': {
case 'DOCUMENT_RECIPIENT_COMPLETED':
case 'RECIPIENT_SIGNED': {
// v1.13 fires DOCUMENT_SIGNED per recipient sign;
// 2.x fires DOCUMENT_RECIPIENT_COMPLETED for the same semantics.
// Some 2.x deployments emit RECIPIENT_SIGNED as a v2-flavoured alias —
// log when we see it (telemetry) and route to the same handler so v2
// deployments don't silently drop per-recipient signs.
if (event === 'RECIPIENT_SIGNED') {
logger.info(
{ event, documensoId },
'Documenso v2 RECIPIENT_SIGNED received — routing to recipient-signed handler',
);
}
const signedRecipients = recipients.filter(
(r) => r.signingStatus === 'SIGNED' || Boolean(r.signedAt),
);
@@ -162,13 +172,23 @@ export async function POST(req: NextRequest): Promise<NextResponse> {
break;
}
case 'DOCUMENT_OPENED': {
case 'DOCUMENT_OPENED':
case 'RECIPIENT_VIEWED': {
// Documenso v1 sends `readStatus: 'OPENED'`; v2 has used both
// upper and lower case across releases and may omit the field
// entirely (the event itself signals the open). Treat the event
// as the signal: dispatch a per-recipient open for every
// recipient on the document so v2 deployments stop silently
// dropping opens.
//
// RECIPIENT_VIEWED is the v2-flavoured alias for the same semantics
// — log when we see it (telemetry) and route to the same handler.
if (event === 'RECIPIENT_VIEWED') {
logger.info(
{ event, documensoId },
'Documenso v2 RECIPIENT_VIEWED received — routing to document-opened handler',
);
}
const openedRecipients = recipients.filter(
(r) => !r.readStatus || String(r.readStatus).toUpperCase() === 'OPENED',
);
@@ -187,8 +207,17 @@ export async function POST(req: NextRequest): Promise<NextResponse> {
await handleDocumentCompleted({ documentId: documensoId, ...portScope });
break;
case 'DOCUMENT_REJECTED': {
const rejecting = recipients.find((r) => r.signingStatus === 'REJECTED');
case 'DOCUMENT_REJECTED':
case 'DOCUMENT_DECLINED': {
// Documenso v2 distinguishes Decline (recipient refuses to sign) from
// Reject (admin cancels). Both currently map to the same "rejected"
// terminal state in our domain — `handleDocumentRejected` records who
// refused and freezes the workflow. Product may later refine
// downstream UX (different audit tags / notifications), but the
// storage shape is identical for now so they share a handler.
const rejecting = recipients.find(
(r) => r.signingStatus === 'REJECTED' || r.signingStatus === 'DECLINED',
);
await handleDocumentRejected({
documentId: documensoId,
recipientEmail: rejecting?.email,

View File

@@ -32,14 +32,31 @@ export interface StorageKeyTable {
}
/**
* Phase 6a ships an empty list — `berth_pdf_versions` and `brochure_versions`
* land in Phase 6b. Add new entries here when new file-bearing tables are
* introduced. The migration script reads each named table via raw SQL so it
* does not need to import every domain's Drizzle schema.
* Tables that hold blob references the migration script must walk.
*
* Column naming is intentionally inconsistent across the schema for historical
* reasons:
* - `files.storage_path` (oldest table, named before §4.7a rename)
* - `berth_pdf_versions.storage_key` (Phase 6b — followed the new convention)
* - `brochure_versions.storage_key` (Phase 6b)
* - `gdpr_exports.storage_key` (worker-uploaded export bundle)
*
* None of these tables carry a per-row content-type column today
* (`files.mime_type` exists but isn't the same semantics — it's the
* original-upload mime, not the stored object's Content-Type header). The
* migration falls back to `application/octet-stream` when
* `contentTypeColumn` is omitted; the byte stream is what matters for the
* sha256-verified round-trip and the original Content-Type is already
* persisted on the source object's S3 metadata.
*
* The `report_snapshots` table called out in the audit does not exist yet.
* Add it here when it lands.
*/
export const TABLES_WITH_STORAGE_KEYS: StorageKeyTable[] = [
// { table: 'berth_pdf_versions', keyColumn: 'storage_key', pkColumn: 'id', contentTypeColumn: 'content_type' },
// { table: 'brochure_versions', keyColumn: 'storage_key', pkColumn: 'id', contentTypeColumn: 'content_type' },
{ table: 'files', keyColumn: 'storage_path', pkColumn: 'id' },
{ table: 'berth_pdf_versions', keyColumn: 'storage_key', pkColumn: 'id' },
{ table: 'brochure_versions', keyColumn: 'storage_key', pkColumn: 'id' },
{ table: 'gdpr_exports', keyColumn: 'storage_key', pkColumn: 'id' },
];
const ADVISORY_LOCK_KEY = 0xc7000a01;

View File

@@ -30,6 +30,32 @@ interface S3BackendConfig {
forcePathStyle?: boolean;
}
/**
* Socket timeout wrapper. The `minio` JS client does not propagate
* `fetchWithTimeout` semantics into `putObject` / `getObject` / `statObject`
* (its underlying `node:http(s)` agent has no default request-timeout), so a
* TCP-blackhole between the worker and the storage host can stall a job
* indefinitely. We race every call against a deadline and fail loud — the
* caller's retry/error path is far better than a stuck queue worker.
*
* The MinIO client doesn't accept an AbortSignal on these methods, so the
* underlying request keeps running in the background after timeout. That's
* acceptable here: the alternative is keeping the worker hung forever; the
* underlying socket is closed by Node's keep-alive timeouts on the next
* idle cycle.
*/
const STORAGE_DEFAULT_TIMEOUT_MS = 30_000;
function withTimeout<T>(promise: Promise<T>, ms: number, label: string): Promise<T> {
let timer: NodeJS.Timeout | null = null;
const timeout = new Promise<T>((_, reject) => {
timer = setTimeout(() => reject(new Error(`S3 ${label} timed out after ${ms}ms`)), ms);
});
return Promise.race([promise, timeout]).finally(() => {
if (timer) clearTimeout(timer);
});
}
interface ResolvedConfig {
endpoint: string;
port: number;
@@ -114,10 +140,18 @@ export class S3Backend implements StorageBackend {
// is missing — we'll create it. Otherwise we throw so the boot fails
// fast and the deployment-time misconfig is loud.
try {
const exists = await client.bucketExists(resolved.bucket);
const exists = await withTimeout(
client.bucketExists(resolved.bucket),
STORAGE_DEFAULT_TIMEOUT_MS,
'bucketExists',
);
if (!exists) {
if (process.env.MINIO_AUTO_CREATE_BUCKET === 'true') {
await client.makeBucket(resolved.bucket, resolved.region);
await withTimeout(
client.makeBucket(resolved.bucket, resolved.region),
STORAGE_DEFAULT_TIMEOUT_MS,
'makeBucket',
);
logger.info(
{ bucket: resolved.bucket, endpoint: resolved.endpoint },
'S3 bucket auto-created (MINIO_AUTO_CREATE_BUCKET=true)',
@@ -153,16 +187,24 @@ export class S3Backend implements StorageBackend {
const buffer = Buffer.isBuffer(body) ? body : await streamToBuffer(body);
const sha256 = opts.sha256 ?? createHash('sha256').update(buffer).digest('hex');
await this.client.putObject(this.bucket, key, buffer, buffer.length, {
'Content-Type': opts.contentType,
});
await withTimeout(
this.client.putObject(this.bucket, key, buffer, buffer.length, {
'Content-Type': opts.contentType,
}),
STORAGE_DEFAULT_TIMEOUT_MS,
`putObject(${key})`,
);
return { key, sizeBytes: buffer.length, sha256 };
}
async get(key: string): Promise<NodeJS.ReadableStream> {
try {
return await this.client.getObject(this.bucket, key);
return await withTimeout(
this.client.getObject(this.bucket, key),
STORAGE_DEFAULT_TIMEOUT_MS,
`getObject(${key})`,
);
} catch (err) {
const code = (err as { code?: string } | null)?.code;
if (code === 'NoSuchKey' || code === 'NotFound') {
@@ -174,7 +216,11 @@ export class S3Backend implements StorageBackend {
async head(key: string): Promise<{ sizeBytes: number; contentType: string } | null> {
try {
const stat = await this.client.statObject(this.bucket, key);
const stat = await withTimeout(
this.client.statObject(this.bucket, key),
STORAGE_DEFAULT_TIMEOUT_MS,
`statObject(${key})`,
);
const meta = (stat.metaData ?? {}) as Record<string, string>;
const contentType =
meta['content-type'] ?? meta['Content-Type'] ?? 'application/octet-stream';
@@ -188,7 +234,11 @@ export class S3Backend implements StorageBackend {
async delete(key: string): Promise<void> {
try {
await this.client.removeObject(this.bucket, key);
await withTimeout(
this.client.removeObject(this.bucket, key),
STORAGE_DEFAULT_TIMEOUT_MS,
`removeObject(${key})`,
);
} catch (err) {
const code = (err as { code?: string } | null)?.code;
if (code === 'NotFound' || code === 'NoSuchKey') return;
@@ -232,14 +282,26 @@ export class S3Backend implements StorageBackend {
const sentinelKey = `_health/${Date.now()}.txt`;
const payload = Buffer.from('ok', 'utf8');
try {
await this.client.putObject(this.bucket, sentinelKey, payload, payload.length, {
'Content-Type': 'text/plain',
});
const stat = await this.client.statObject(this.bucket, sentinelKey);
await withTimeout(
this.client.putObject(this.bucket, sentinelKey, payload, payload.length, {
'Content-Type': 'text/plain',
}),
STORAGE_DEFAULT_TIMEOUT_MS,
`healthCheck:put(${sentinelKey})`,
);
const stat = await withTimeout(
this.client.statObject(this.bucket, sentinelKey),
STORAGE_DEFAULT_TIMEOUT_MS,
`healthCheck:stat(${sentinelKey})`,
);
if (stat.size !== payload.length) {
return { ok: false, error: 'sentinel size mismatch' };
}
await this.client.removeObject(this.bucket, sentinelKey);
await withTimeout(
this.client.removeObject(this.bucket, sentinelKey),
STORAGE_DEFAULT_TIMEOUT_MS,
`healthCheck:remove(${sentinelKey})`,
);
return { ok: true };
} catch (err) {
return { ok: false, error: (err as Error).message };

View File

@@ -94,14 +94,15 @@ test.describe('Documents hub — aggregated view', () => {
email: `hubagg${Date.now()}@e2e.test`,
},
});
// If creation fails (e.g. no active port cookie yet), skip gracefully —
// we still assert basic hub structure in the earlier tests.
// A non-2xx here means smoke setup is broken (port cookie / seed) or the
// clients API regressed. Fail loud rather than skip green — a silent skip
// masked an infra failure for weeks in the audit window. Playwright doesn't
// expose vitest's `expect.fail`, so we throw a plain Error which the
// runner promotes to a failing test the same way.
if (!res.ok()) {
test.skip(
true,
`Client create returned ${res.status()} — entity sub-folder assertion skipped`,
throw new Error(
`Client create returned ${res.status()} ${await res.text()} — entity sub-folder assertion cannot proceed`,
);
return;
}
const { data: client } = (await res.json()) as {
data: { id: string; firstName: string; lastName: string };

View File

@@ -37,9 +37,11 @@ test.describe('Documents hub — upload into entity folder', () => {
email: `uploadsmoke${Date.now()}@e2e.test`,
},
});
// Playwright doesn't expose vitest's `expect.fail`; throw to fail loud.
if (!clientRes.ok()) {
test.skip(true, `Client create returned ${clientRes.status()} — upload test skipped`);
return;
throw new Error(
`Client create returned ${clientRes.status()} ${await clientRes.text()} — upload test cannot proceed`,
);
}
const { data: client } = (await clientRes.json()) as {
data: { id: string; firstName: string; lastName: string };
@@ -126,8 +128,9 @@ test.describe('Documents hub — upload into entity folder', () => {
},
});
if (!clientRes.ok()) {
test.skip(true, `Client create returned ${clientRes.status()} — test skipped`);
return;
throw new Error(
`Client create returned ${clientRes.status()} ${await clientRes.text()} — folderId test cannot proceed`,
);
}
const { data: client } = (await clientRes.json()) as {
data: { id: string; firstName: string; lastName: string };
@@ -148,10 +151,12 @@ test.describe('Documents hub — upload into entity folder', () => {
clientId: client.id,
},
});
// Seed upload may fail if files module isn't fully wired — skip gracefully.
// Seed upload failing means the files API is broken — fail loud so the
// infra regression surfaces in CI instead of staying green-skipped.
if (!seedUpload.ok()) {
test.skip(true, `Seed upload returned ${seedUpload.status()} — folderId test skipped`);
return;
throw new Error(
`Seed upload returned ${seedUpload.status()} ${await seedUpload.text()} — folderId test cannot proceed`,
);
}
// 3. List files for this client to discover the folder id.
@@ -162,8 +167,9 @@ test.describe('Documents hub — upload into entity folder', () => {
},
);
if (!listRes.ok()) {
test.skip(true, `File list returned ${listRes.status()} — folderId test skipped`);
return;
throw new Error(
`File list returned ${listRes.status()} ${await listRes.text()} — folderId test cannot proceed`,
);
}
// 4. Navigate and verify — folder view shows the client entity sections.

View File

@@ -0,0 +1,153 @@
/**
* Regression tests for document-folders edge cases surfaced by the
* 2026-05-11 prod-readiness audit.
*
* Covers:
* - `applyEntityRestoredSuffix` no-op when the folder was never archived
* (must not flip archived_at, must not rename anything, must not emit
* an audit log).
* - `syncEntityFolderName` collision loop past `(2)` — proves the suffix
* loop iterates correctly when the first numbered candidate is also
* taken. Existing coverage only asserted the `(2)` case.
*
* The audit also calls out a `mapWorkflowStatus` unit test for the
* `partially_signed → 'partial'` mapping, but that helper currently lives
* inside React component files (`entity-folder-view.tsx`,
* `signing-details-dialog.tsx`, `documents-hub.tsx`) and is not exported.
* A real unit test would require extracting it to a shared util — out of
* scope for this subagent's file ownership. See the audit report for the
* deferred fix.
*/
import { describe, it, expect, beforeAll, beforeEach } from 'vitest';
import { and, eq } from 'drizzle-orm';
import { db } from '@/lib/db';
import { documentFolders } from '@/lib/db/schema/documents';
import { clients } from '@/lib/db/schema/clients';
import { user } from '@/lib/db/schema/users';
import {
ensureSystemRoots,
ensureEntityFolder,
applyEntityRestoredSuffix,
syncEntityFolderName,
} from '@/lib/services/document-folders.service';
import { makePort } from '../helpers/factories';
let TEST_USER_ID = '';
beforeAll(async () => {
const [u] = await db.select({ id: user.id }).from(user).limit(1);
if (!u) throw new Error('No user available; run pnpm db:seed first');
TEST_USER_ID = u.id;
});
describe('document-folders · applyEntityRestoredSuffix no-op (regression)', () => {
let portId: string;
let clientId: string;
let originalName: string;
beforeEach(async () => {
const port = await makePort();
portId = port.id;
await db.delete(documentFolders).where(eq(documentFolders.portId, portId));
await ensureSystemRoots(portId, TEST_USER_ID);
originalName = `Restore Probe ${crypto.randomUUID().slice(0, 6)}`;
const [c] = await db.insert(clients).values({ portId, fullName: originalName }).returning();
clientId = c!.id;
await ensureEntityFolder(portId, 'client', clientId, TEST_USER_ID);
});
it('is a no-op when the folder was never archived (name unchanged, archivedAt stays null)', async () => {
const before = await db.query.documentFolders.findFirst({
where: and(eq(documentFolders.entityType, 'client'), eq(documentFolders.entityId, clientId)),
});
expect(before?.archivedAt).toBeNull();
expect(before?.name).toBe(originalName);
await applyEntityRestoredSuffix(portId, 'client', clientId);
const after = await db.query.documentFolders.findFirst({
where: and(eq(documentFolders.entityType, 'client'), eq(documentFolders.entityId, clientId)),
});
expect(after?.name).toBe(originalName);
expect(after?.archivedAt).toBeNull();
// updatedAt should not advance on a no-op restore — the row write is
// skipped entirely.
expect(after?.updatedAt?.getTime()).toBe(before?.updatedAt?.getTime());
});
it('is a no-op when called for an entity whose folder does not exist (lazy creation)', async () => {
// Different port — no folder for this client.
const otherPort = await makePort();
await ensureSystemRoots(otherPort.id, TEST_USER_ID);
const [other] = await db
.insert(clients)
.values({ portId: otherPort.id, fullName: `Lazy ${crypto.randomUUID().slice(0, 6)}` })
.returning();
await expect(
applyEntityRestoredSuffix(otherPort.id, 'client', other!.id),
).resolves.toBeUndefined();
// No folder should have been created as a side-effect.
const rows = await db
.select()
.from(documentFolders)
.where(
and(
eq(documentFolders.portId, otherPort.id),
eq(documentFolders.entityType, 'client'),
eq(documentFolders.entityId, other!.id),
),
);
expect(rows).toHaveLength(0);
});
});
describe('document-folders · syncEntityFolderName collision loop > (2) (regression)', () => {
let portId: string;
beforeEach(async () => {
const port = await makePort();
portId = port.id;
await db.delete(documentFolders).where(eq(documentFolders.portId, portId));
await ensureSystemRoots(portId, TEST_USER_ID);
});
it('walks past (2) → (3) when the (2) suffix is also taken', async () => {
// Three clients with the same name — first two are pre-created with their
// entity folders so `sharedName` and `sharedName (2)` are both occupied
// before we trigger the rename on the third.
const sharedName = `Triple Collision ${crypto.randomUUID().slice(0, 6)}`;
const [first] = await db.insert(clients).values({ portId, fullName: sharedName }).returning();
await ensureEntityFolder(portId, 'client', first!.id, TEST_USER_ID);
const [second] = await db.insert(clients).values({ portId, fullName: sharedName }).returning();
const secondFolder = await ensureEntityFolder(portId, 'client', second!.id, TEST_USER_ID);
// Sanity — second client's folder is the "(2)" variant.
expect(secondFolder.name).toBe(`${sharedName} (2)`);
// Third client — start with a different name so its folder is unique,
// then rename it to the shared name to force `syncEntityFolderName` to
// walk past (2).
const placeholderName = `Triple Collision Placeholder ${crypto.randomUUID().slice(0, 6)}`;
const [third] = await db
.insert(clients)
.values({ portId, fullName: placeholderName })
.returning();
await ensureEntityFolder(portId, 'client', third!.id, TEST_USER_ID);
// Rename the entity → sync should pick `${sharedName} (3)` after seeing
// both `${sharedName}` and `${sharedName} (2)` are taken.
await db.update(clients).set({ fullName: sharedName }).where(eq(clients.id, third!.id));
await syncEntityFolderName(portId, 'client', third!.id, TEST_USER_ID);
const folder = await db.query.documentFolders.findFirst({
where: and(eq(documentFolders.entityType, 'client'), eq(documentFolders.entityId, third!.id)),
});
expect(folder?.name).toBe(`${sharedName} (3)`);
});
});

View File

@@ -5,6 +5,7 @@ import {
moveFolderSchema,
moveDocumentToFolderSchema,
} from '@/lib/validators/document-folders';
import { listDocumentsSchema } from '@/lib/validators/documents';
describe('document-folder validators', () => {
it('accepts a valid create payload', () => {
@@ -37,4 +38,49 @@ describe('document-folder validators', () => {
expect(moveDocumentToFolderSchema.safeParse({ folderId: null }).success).toBe(true);
expect(moveDocumentToFolderSchema.safeParse({ folderId: 'abc' }).success).toBe(true);
});
// ─── folderId='' → null transform (regression) ─────────────────────────────
//
// The frontend's URL-query builder emits `?folderId=` (empty string) when
// the user picks "All documents" — without the transform, Zod would parse
// this as the literal string "" and the SQL layer would try to JOIN on an
// empty folder id, returning zero rows instead of the expected unscoped
// result. The transform lives on `listDocumentsSchema` (and
// `listFilesSchema`); we exercise the documents one here.
describe("listDocumentsSchema folderId='' transform", () => {
const BASE = { page: 1, limit: 20, sort: 'createdAt', order: 'desc', includeArchived: 'false' };
it("coerces folderId='' to null so the empty-string query param means 'unscoped'", () => {
const result = listDocumentsSchema.safeParse({ ...BASE, folderId: '' });
expect(result.success).toBe(true);
if (result.success) {
expect(result.data.folderId).toBeNull();
}
});
it('leaves a non-empty folderId untouched', () => {
const id = crypto.randomUUID();
const result = listDocumentsSchema.safeParse({ ...BASE, folderId: id });
expect(result.success).toBe(true);
if (result.success) {
expect(result.data.folderId).toBe(id);
}
});
it('accepts an explicit folderId=null (no transform needed)', () => {
const result = listDocumentsSchema.safeParse({ ...BASE, folderId: null });
expect(result.success).toBe(true);
if (result.success) {
expect(result.data.folderId).toBeNull();
}
});
it('treats an absent folderId as undefined (not the same as null)', () => {
const result = listDocumentsSchema.safeParse(BASE);
expect(result.success).toBe(true);
if (result.success) {
expect(result.data.folderId).toBeUndefined();
}
});
});
});