diff --git a/apps/mobile-rn/__tests__/generation-key-lifecycle-r2-29.test.ts b/apps/mobile-rn/__tests__/generation-key-lifecycle-r2-29.test.ts new file mode 100644 index 0000000..b23a246 --- /dev/null +++ b/apps/mobile-rn/__tests__/generation-key-lifecycle-r2-29.test.ts @@ -0,0 +1,226 @@ +jest.mock('../src/lib/supabase', () => ({ + supabase: { + rpc: jest.fn(), + from: jest.fn(), + channel: jest.fn(), + removeChannel: jest.fn(), + }, +})) + +import AsyncStorage from '@react-native-async-storage/async-storage' +import type { MeetingDocument } from '@d3ro/api-client' +import { + generationKeyDisposition, + type GenerationAttemptOutcome, + type GenerationKeyDisposition, +} from '../src/features/templates/generation-key-policy' +import { getOrCreateGenerationIdempotencyKey } from '../src/features/templates/generation-idempotency' +import { + generateMeetingDocumentOnce, + generationAttemptOutcome, + TemplateServiceError, + type GenerateMeetingDocumentOnceDeps, +} from '../src/features/templates' +import type { GenerateMeetingDocumentOptions } from '../src/features/templates' + +const USER_ID = '11111111-1111-4111-8111-111111111111' +const MEETING_ID = '33333333-3333-4333-8333-333333333333' +const TEMPLATE_ID = '44444444-4444-4444-8444-444444444444' +const DOCUMENT_ID = '55555555-5555-4555-8555-555555555555' +const ACCESS_TOKEN = 'access-token-with-enough-length' + +function response(status: number, errorCode: string | null): GenerationAttemptOutcome { + return { kind: 'server-response', status, errorCode } +} + +describe('generationKeyDisposition', () => { + const table: Array<[string, GenerationAttemptOutcome, GenerationKeyDisposition]> = [ + ['success', { kind: 'succeeded' }, 'discard'], + ['idempotent 200 replay', response(200, null), 'discard'], + ['no response (network loss / abort / local pre-flight)', { kind: 'no-response' }, 'retain'], + ['400 idempotency conflict or invalid request', response(400, 'invalid_request'), 'discard'], + ['401 session expired before claim', response(401, 'unauthorized'), 'retain'], + ['403 forbidden', response(403, 'forbidden'), 'discard'], + ['404 not found', response(404, 'not_found'), 'discard'], + ['409 generation_failed', response(409, 'generation_failed'), 'discard'], + ['409 generation_in_progress', response(409, 'generation_in_progress'), 'retain'], + ['409 without body', response(409, null), 'retain'], + ['429 quota exceeded', response(429, 'quota_exceeded'), 'discard'], + ['500 commit_failed (row marked failed)', response(500, 'commit_failed'), 'discard'], + ['500 internal_error (state unknown)', response(500, 'internal_error'), 'retain'], + ['502 provider_request_failed', response(502, 'provider_request_failed'), 'discard'], + ['502 provider_invalid_response', response(502, 'provider_invalid_response'), 'discard'], + ['504 provider_timeout', response(504, 'provider_timeout'), 'discard'], + ['504 gateway timeout without function body', response(504, null), 'retain'], + ['503 unknown gateway error', response(503, 'service_unavailable'), 'retain'], + ] + + it.each(table)('%s -> %s', (_label, outcome, expected) => { + expect(generationKeyDisposition(outcome)).toBe(expected) + }) +}) + +describe('generationAttemptOutcome', () => { + it('uses the server response carried by the service error', () => { + const error = new TemplateServiceError('provider', 'provider_timeout', true, { + status: 504, + errorCode: 'provider_timeout', + }) + expect(generationAttemptOutcome(error)).toEqual(response(504, 'provider_timeout')) + }) + + it('treats errors without a server response as no response', () => { + expect(generationAttemptOutcome(new TemplateServiceError('network', 'Network request failed', true))) + .toEqual({ kind: 'no-response' }) + expect(generationAttemptOutcome(new TypeError('Network request failed'))) + .toEqual({ kind: 'no-response' }) + }) +}) + +describe('generateMeetingDocumentOnce', () => { + const fetchMock = jest.fn() + const originalFetch = global.fetch + + function jsonResponse(status: number, body: unknown): Response { + return { + ok: status >= 200 && status < 300, + status, + json: async () => body, + } as unknown as Response + } + + function sentKey(callIndex: number): string { + const init = fetchMock.mock.calls[callIndex]?.[1] as { body: string } + return (JSON.parse(init.body) as { idempotencyKey: string }).idempotencyKey + } + + const document = { + id: DOCUMENT_ID, + meeting_id: MEETING_ID, + template_id: TEMPLATE_ID, + } as unknown as MeetingDocument + + const options = { + userId: USER_ID, + accessToken: ACCESS_TOKEN, + meetingId: MEETING_ID, + templateId: TEMPLATE_ID, + title: 'Weekly sync · Minutes', + } + + beforeEach(async () => { + await AsyncStorage.clear() + fetchMock.mockReset() + global.fetch = fetchMock as unknown as typeof fetch + }) + + afterAll(() => { + global.fetch = originalFetch + }) + + it('starts a fresh operation after the server marks the request failed (provider timeout)', async () => { + fetchMock + .mockResolvedValueOnce(jsonResponse(504, { error: 'provider_timeout' })) + .mockResolvedValueOnce(jsonResponse(200, { document, idempotent: false })) + + await expect(generateMeetingDocumentOnce(options)).rejects.toMatchObject({ code: 'provider' }) + const generated = await generateMeetingDocumentOnce(options) + + expect(generated.document.id).toBe(DOCUMENT_ID) + expect(sentKey(1)).not.toBe(sentKey(0)) + }) + + it('starts a fresh operation after an idempotency hash conflict', async () => { + fetchMock + .mockResolvedValueOnce(jsonResponse(400, { error: 'invalid_request' })) + .mockResolvedValueOnce(jsonResponse(200, { document, idempotent: false })) + + await expect(generateMeetingDocumentOnce(options)).rejects.toMatchObject({ code: 'validation' }) + await generateMeetingDocumentOnce(options) + + expect(sentKey(1)).not.toBe(sentKey(0)) + }) + + it('reports 409 generation_failed as a retryable server error and discards the key', async () => { + fetchMock + .mockResolvedValueOnce(jsonResponse(409, { error: 'generation_failed' })) + .mockResolvedValueOnce(jsonResponse(200, { document, idempotent: false })) + + await expect(generateMeetingDocumentOnce(options)) + .rejects.toMatchObject({ code: 'server', retryable: true }) + await generateMeetingDocumentOnce(options) + + expect(sentKey(1)).not.toBe(sentKey(0)) + }) + + it('reuses the key while the server is still processing it', async () => { + fetchMock + .mockResolvedValueOnce(jsonResponse(409, { error: 'generation_in_progress' })) + .mockResolvedValueOnce(jsonResponse(200, { document, idempotent: true })) + + await expect(generateMeetingDocumentOnce(options)).rejects.toMatchObject({ code: 'in-progress' }) + await generateMeetingDocumentOnce(options) + + expect(sentKey(1)).toBe(sentKey(0)) + }) + + it('reuses the key when no response arrived (network loss or abort)', async () => { + fetchMock + .mockRejectedValueOnce(new TypeError('Network request failed')) + .mockResolvedValueOnce(jsonResponse(200, { document, idempotent: true })) + + await expect(generateMeetingDocumentOnce(options)).rejects.toMatchObject({ code: 'network' }) + await generateMeetingDocumentOnce(options) + + expect(sentKey(1)).toBe(sentKey(0)) + }) + + it('clears the key after success so the next generation is a new operation', async () => { + fetchMock.mockResolvedValue(jsonResponse(200, { document, idempotent: false })) + + await generateMeetingDocumentOnce(options) + await generateMeetingDocumentOnce(options) + + expect(sentKey(1)).not.toBe(sentKey(0)) + }) + + it('keeps the key when local validation fails before any request is sent', async () => { + const pending = await getOrCreateGenerationIdempotencyKey(USER_ID, MEETING_ID, TEMPLATE_ID) + + await expect(generateMeetingDocumentOnce({ ...options, title: ' ' })) + .rejects.toMatchObject({ code: 'validation' }) + + expect(fetchMock).not.toHaveBeenCalled() + expect(await getOrCreateGenerationIdempotencyKey(USER_ID, MEETING_ID, TEMPLATE_ID)).toBe(pending) + }) + + it('returns the result even when clearing the key fails', async () => { + const deps: GenerateMeetingDocumentOnceDeps = { + keyStore: { + getOrCreate: jest.fn(async () => '66666666-6666-4666-8666-666666666666'), + clear: jest.fn(async () => { throw new Error('storage unavailable') }), + }, + generate: jest.fn(async (request: GenerateMeetingDocumentOptions) => { + expect(request.idempotencyKey).toBe('66666666-6666-4666-8666-666666666666') + return { document, idempotent: false } + }), + } + + await expect(generateMeetingDocumentOnce(options, deps)).resolves.toMatchObject({ document }) + expect(deps.keyStore.clear).toHaveBeenCalledWith(USER_ID, MEETING_ID, TEMPLATE_ID) + }) + + it('maps key-store failures to a service error without calling the server', async () => { + const deps: GenerateMeetingDocumentOnceDeps = { + keyStore: { + getOrCreate: jest.fn(async () => { throw new Error('invalid_generation_identity') }), + clear: jest.fn(async () => undefined), + }, + generate: jest.fn(), + } + + await expect(generateMeetingDocumentOnce(options, deps)).rejects.toBeInstanceOf(TemplateServiceError) + expect(deps.generate).not.toHaveBeenCalled() + expect(deps.keyStore.clear).not.toHaveBeenCalled() + }) +}) diff --git a/apps/mobile-rn/src/features/templates/generation-idempotency.ts b/apps/mobile-rn/src/features/templates/generation-idempotency.ts index 019dd8b..a11022a 100644 --- a/apps/mobile-rn/src/features/templates/generation-idempotency.ts +++ b/apps/mobile-rn/src/features/templates/generation-idempotency.ts @@ -48,3 +48,17 @@ export async function clearEveryGenerationIdempotencyKey(): Promise { const keys = (await AsyncStorage.getAllKeys()).filter((key) => key.startsWith(PREFIX)) if (keys.length > 0) await AsyncStorage.multiRemove(keys) } + +/** + * Port for the per (user, meeting, template) pending-generation key. The + * generation use case depends on this interface, not on AsyncStorage. + */ +export interface GenerationKeyStore { + getOrCreate: (userId: string, meetingId: string, templateId: string) => Promise + clear: (userId: string, meetingId: string, templateId: string) => Promise +} + +export const asyncStorageGenerationKeyStore: GenerationKeyStore = { + getOrCreate: getOrCreateGenerationIdempotencyKey, + clear: clearGenerationIdempotencyKey, +} diff --git a/apps/mobile-rn/src/features/templates/generation-key-policy.ts b/apps/mobile-rn/src/features/templates/generation-key-policy.ts new file mode 100644 index 0000000..44b6cfd --- /dev/null +++ b/apps/mobile-rn/src/features/templates/generation-key-policy.ts @@ -0,0 +1,55 @@ +/** + * Idempotency-key lifecycle policy for meeting document generation. + * + * The server binds an idempotency key to exactly one generation request row. + * Once that row reaches a final state ('succeeded' or 'failed'), or the server + * refuses the key outright (hash conflict, quota, permission), reusing the key + * can only ever replay that final answer. The key must therefore be discarded + * so the next user attempt starts a fresh operation. + * + * The key is retained only while the server's state for it is unknown or still + * running: no response reached the client (network loss, abort, local + * pre-flight failure), the server reports the request is still processing, or + * the response does not come from the generation function itself (an unknown + * 5xx such as a gateway timeout, while the worker may still commit). Retrying + * with the same key then converges on the server's real outcome instead of + * spending a second generation. + * + * Pure and IO-free so the full status/error-code matrix can be table-tested. + */ + +export type GenerationAttemptOutcome = + | { kind: 'succeeded' } + | { kind: 'server-response'; status: number; errorCode: string | null } + | { kind: 'no-response' } + +export type GenerationKeyDisposition = 'discard' | 'retain' + +/** 5xx codes the generation function emits only after marking the row failed. */ +const TERMINAL_SERVER_FAILURE_CODES: ReadonlySet = new Set([ + 'provider_timeout', + 'provider_request_failed', + 'provider_invalid_response', + 'commit_failed', +]) + +/** 4xx statuses where the server has refused or finalized this key. */ +const TERMINAL_CLIENT_ERROR_STATUSES: ReadonlySet = new Set([400, 403, 404, 429]) + +export function generationKeyDisposition( + outcome: GenerationAttemptOutcome, +): GenerationKeyDisposition { + if (outcome.kind === 'succeeded') return 'discard' + if (outcome.kind === 'no-response') return 'retain' + + const { status, errorCode } = outcome + if (status >= 200 && status < 300) return 'discard' + if (status === 409) return errorCode === 'generation_failed' ? 'discard' : 'retain' + if (TERMINAL_CLIENT_ERROR_STATUSES.has(status)) return 'discard' + if (status >= 500 && errorCode !== null && TERMINAL_SERVER_FAILURE_CODES.has(errorCode)) { + return 'discard' + } + // 401 (session expired before the claim ran), 500 internal_error and any + // gateway-produced 5xx leave the key's server state unknown. + return 'retain' +} diff --git a/apps/mobile-rn/src/features/templates/template-service.ts b/apps/mobile-rn/src/features/templates/template-service.ts index e1a5a82..0f042db 100644 --- a/apps/mobile-rn/src/features/templates/template-service.ts +++ b/apps/mobile-rn/src/features/templates/template-service.ts @@ -9,6 +9,14 @@ import type { } from '@d3ro/api-client' import { SUPABASE_URL } from '@d3ro/core/supabase-config' import { supabase } from '../../lib/supabase' +import { + asyncStorageGenerationKeyStore, + type GenerationKeyStore, +} from './generation-idempotency' +import { + generationKeyDisposition, + type GenerationAttemptOutcome, +} from './generation-key-policy' import type { GenerateMeetingDocumentOptions, GeneratedMeetingDocument, @@ -29,11 +37,18 @@ export type TemplateServiceErrorCode = | 'server' | 'validation' +/** The HTTP answer the server gave, when an error came from a server response. */ +export interface TemplateServiceResponseInfo { + status: number + errorCode: string | null +} + export class TemplateServiceError extends Error { constructor( public readonly code: TemplateServiceErrorCode, message: string, public readonly retryable = false, + public readonly response: TemplateServiceResponseInfo | null = null, ) { super(message) this.name = 'TemplateServiceError' @@ -357,14 +372,21 @@ export function renderDictationTemplate( function generationError(status: number, body: unknown): TemplateServiceError { const code = isRecord(body) && typeof body.error === 'string' ? body.error : '' - if (status === 401) return new TemplateServiceError('auth', code) - if (status === 403) return new TemplateServiceError('forbidden', code) - if (status === 404) return new TemplateServiceError('not-found', code) - if (status === 409) return new TemplateServiceError('in-progress', code, true) - if (status === 429) return new TemplateServiceError('quota', code, true) - if ([502, 503, 504].includes(status)) return new TemplateServiceError('provider', code, true) - if (status === 400) return new TemplateServiceError('validation', code) - return new TemplateServiceError('server', code || 'Document generation failed', true) + const response: TemplateServiceResponseInfo = { status, errorCode: code || null } + if (status === 401) return new TemplateServiceError('auth', code, false, response) + if (status === 403) return new TemplateServiceError('forbidden', code, false, response) + if (status === 404) return new TemplateServiceError('not-found', code, false, response) + if (status === 409 && code === 'generation_failed') { + // The previous attempt with this key failed for good; a new attempt can succeed. + return new TemplateServiceError('server', code, true, response) + } + if (status === 409) return new TemplateServiceError('in-progress', code, true, response) + if (status === 429) return new TemplateServiceError('quota', code, true, response) + if ([502, 503, 504].includes(status)) { + return new TemplateServiceError('provider', code, true, response) + } + if (status === 400) return new TemplateServiceError('validation', code, false, response) + return new TemplateServiceError('server', code || 'Document generation failed', true, response) } function isRecord(value: unknown): value is Record { @@ -416,6 +438,71 @@ export async function generateMeetingDocument( } } +export interface GenerateMeetingDocumentOnceOptions + extends Omit { + userId: string +} + +export interface GenerateMeetingDocumentOnceDeps { + keyStore: GenerationKeyStore + generate: (options: GenerateMeetingDocumentOptions) => Promise +} + +const DEFAULT_GENERATE_ONCE_DEPS: GenerateMeetingDocumentOnceDeps = { + keyStore: asyncStorageGenerationKeyStore, + generate: generateMeetingDocument, +} + +/** Maps a failed attempt to what the client actually learned from the server. */ +export function generationAttemptOutcome(error: unknown): GenerationAttemptOutcome { + if (error instanceof TemplateServiceError && error.response !== null) { + return { + kind: 'server-response', + status: error.response.status, + errorCode: error.response.errorCode, + } + } + return { kind: 'no-response' } +} + +/** + * Generates one meeting document and owns its idempotency key's lifecycle: + * the key for (user, meeting, template) is reused across ambiguous retries and + * discarded once the server has given a final answer for it, success or not. + */ +export async function generateMeetingDocumentOnce( + options: GenerateMeetingDocumentOnceOptions, + deps: GenerateMeetingDocumentOnceDeps = DEFAULT_GENERATE_ONCE_DEPS, +): Promise { + const { userId, ...request } = options + let idempotencyKey: string + try { + idempotencyKey = await deps.keyStore.getOrCreate(userId, request.meetingId, request.templateId) + } catch (error) { + throw toServiceError(error) + } + + let outcome: GenerationAttemptOutcome + let generated: GeneratedMeetingDocument | null = null + let failure: unknown = null + try { + generated = await deps.generate({ ...request, idempotencyKey }) + outcome = { kind: 'succeeded' } + } catch (error) { + failure = error + outcome = generationAttemptOutcome(error) + } + + if (generationKeyDisposition(outcome) === 'discard') { + // A failed cleanup only costs one extra replay of the final answer. + await deps.keyStore + .clear(userId, request.meetingId, request.templateId) + .catch(() => undefined) + } + if (generated === null) throw toServiceError(failure) + return generated +} + export function subscribeToTemplates( userId: string, onRemoteChange: () => void, diff --git a/apps/mobile-rn/src/screens/MeetingDetailScreen.tsx b/apps/mobile-rn/src/screens/MeetingDetailScreen.tsx index 4eb4ad3..fcbc777 100644 --- a/apps/mobile-rn/src/screens/MeetingDetailScreen.tsx +++ b/apps/mobile-rn/src/screens/MeetingDetailScreen.tsx @@ -29,9 +29,7 @@ import { type PreparedPortableFile, } from '../features/data-portability' import { - clearGenerationIdempotencyKey, - generateMeetingDocument, - getOrCreateGenerationIdempotencyKey, + generateMeetingDocumentOnce, loadTemplateLibrary, TemplateServiceError, } from '../features/templates' @@ -417,18 +415,13 @@ export default function MeetingDetailScreen({ setBusyKey('document-generate') setError(null) try { - const idempotencyKey = await getOrCreateGenerationIdempotencyKey( - user.id, - meetingId, - templateId, - ) const title = `${detail.meeting.title?.trim() || t('mobile.meetings.untitled')} · ${selectedDocumentTemplate.name}` .slice(0, 160) - const generated = await generateMeetingDocument({ + const generated = await generateMeetingDocumentOnce({ + userId: user.id, accessToken: session.access_token, meetingId, templateId, - idempotencyKey, title, }) setDetail((current) => { @@ -444,7 +437,6 @@ export default function MeetingDetailScreen({ } }) setExpandedDocumentId(generated.document.id) - await clearGenerationIdempotencyKey(user.id, meetingId, templateId).catch(() => undefined) } catch (requestError) { setError(templateErrorMessage(requestError, t)) } finally {