fix(ai-chat): целостность delta-spec (молчаливый скип) + 2 hardening (#491)
Внутреннее ре-ревью: DB-backed delta-spec молча СКИПАЛСЯ — нулевое покрытие
инварианта DB-clock курсора.
- Импорт `import postgres from 'postgres'` (default) при tsconfig
module:commonjs без esModuleInterop компилился в `postgres_1.default(...)`,
а CJS-`postgres` не имеет `.default` → TypeError в beforeAll → пустой catch →
reachable=false → все 6 тестов уходили в console.warn('SKIP') и return БЕЗ
ассертов (suite оставался бы зелёным даже при регрессии на new Date()).
Фикс: `import * as postgres from 'postgres'` (как в рабочем int-харнессе).
- Хардненг харнесса: реальная ошибка программирования в beforeAll больше НЕ
маскируется под «DB unreachable» — скип легитимен только для сетевого отказа
(ECONNREFUSED и т.п.), иначе rethrow → suite падает громко.
- Два DB-clock теста использовали jest.useFakeTimers() целиком, что замораживало
внутренние таймеры postgres.js → awaited DB round-trip зависал на 5s-капе (и
вешал afterAll). Фейкаем ТОЛЬКО Date (doNotFake всех таймеров) — запрос
резолвится, а инвариант «стамп от часов БД, не app-clock» по-прежнему доказан
(скос процесс-часов в 2099 → стамп остаётся на времени БД). Теперь все 6
тестов РЕАЛЬНО исполняются и зелёные против живого Postgres.
Два дешёвых hardening из ревью:
- registry coverageFloor: пустая ветка возвращает max(currentStamp,
persistedFloor) — инвариант «клиент с n=persistedFloor всегда покрыт»
структурный, а не тайминг-зависимый.
- GetChatDeltaDto.cursor: @IsString → @IsISO8601 — битый курсор отсекается 400
на уровне DTO, а не 500 на `::timestamptz`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -440,7 +440,13 @@ export class AiChatStreamRegistryService implements OnModuleDestroy {
|
||||
* An attach at frontier `n` is covered ⟺ coverageFloor <= n.
|
||||
*/
|
||||
private coverageFloor(entry: Entry): number {
|
||||
if (entry.frames.length === 0) return entry.currentStamp;
|
||||
// Empty ring: only the live tail is coming. The floor is the current step,
|
||||
// but never below persistedFloor — a confirmed persist can rotate the ring
|
||||
// empty while currentStamp still lags a beat behind on another connection, so
|
||||
// max() keeps the invariant STRUCTURAL (a client with n = persistedFloor is
|
||||
// always covered) rather than timing-dependent.
|
||||
if (entry.frames.length === 0)
|
||||
return Math.max(entry.currentStamp, entry.persistedFloor);
|
||||
const min = entry.stamps[0];
|
||||
return entry.overflowThroughStamp >= min ? min + 1 : min;
|
||||
}
|
||||
|
||||
@@ -1,4 +1,10 @@
|
||||
import { IsOptional, IsString, MaxLength, MinLength } from 'class-validator';
|
||||
import {
|
||||
IsISO8601,
|
||||
IsOptional,
|
||||
IsString,
|
||||
MaxLength,
|
||||
MinLength,
|
||||
} from 'class-validator';
|
||||
|
||||
/** Identify a chat by id (workspace-scoped on the server). */
|
||||
export class ChatIdDto {
|
||||
@@ -47,8 +53,11 @@ export class GetChatDeltaDto {
|
||||
@IsString()
|
||||
chatId: string;
|
||||
|
||||
// ISO-8601 timestamp echoed from the previous poll's response. Validated as
|
||||
// ISO-8601 (not a bare string): a malformed cursor would otherwise reach the
|
||||
// `::timestamptz` cast in findByChatUpdatedAfter and 500 instead of a clean 400.
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@IsISO8601()
|
||||
cursor?: string;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,7 +1,12 @@
|
||||
import { randomUUID } from 'crypto';
|
||||
import { CamelCasePlugin, Kysely, sql } from 'kysely';
|
||||
import { PostgresJSDialect } from 'kysely-postgres-js';
|
||||
import postgres from 'postgres';
|
||||
// NOT a default import: the project tsconfig is `module: commonjs` with NO
|
||||
// esModuleInterop, so `import postgres from 'postgres'` compiles to
|
||||
// `postgres_1.default(...)` and the CJS `postgres` export has no `.default` —
|
||||
// it threw in beforeAll, was swallowed as "DB unreachable", and SILENTLY voided
|
||||
// all six tests. Mirror the working integration harness (test/integration/db.ts).
|
||||
import * as postgres from 'postgres';
|
||||
import { AiChatMessageRepo } from './ai-chat-message.repo';
|
||||
import { AiChatRunRepo } from './ai-chat-run.repo';
|
||||
|
||||
@@ -50,8 +55,21 @@ beforeAll(async () => {
|
||||
await sql`set session_replication_role = replica`.execute(db);
|
||||
await sql`select 1`.execute(db);
|
||||
reachable = true;
|
||||
} catch {
|
||||
} catch (err) {
|
||||
reachable = false;
|
||||
// A genuine connection failure (ECONNREFUSED etc.) is a legitimate skip on a
|
||||
// DB-less CI. A PROGRAMMING error (bad import, typo, driver misuse) must NOT
|
||||
// masquerade as "DB unreachable" and silently void the whole suite (that is
|
||||
// exactly the bug that hid this spec's zero coverage) — rethrow it so the
|
||||
// suite fails LOUDLY.
|
||||
const msg = String((err as Error)?.message ?? err);
|
||||
if (
|
||||
!/ECONNREFUSED|ENOTFOUND|ETIMEDOUT|EHOSTUNREACH|connect|terminating|password|authentication|role .* does not exist|database .* does not exist/i.test(
|
||||
msg,
|
||||
)
|
||||
) {
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
msgRepo = new AiChatMessageRepo(db as never);
|
||||
runRepo = new AiChatRunRepo(db as never);
|
||||
@@ -96,6 +114,34 @@ async function dbNow(): Promise<string> {
|
||||
return r.rows[0].now.toISOString();
|
||||
}
|
||||
|
||||
// Fake ONLY the Date object (so in-process `new Date()`/`Date.now()` jump), while
|
||||
// leaving every TIMER function real. Faking timers wholesale freezes postgres.js's
|
||||
// internal connection/query timers, so the awaited DB round-trip would hang the
|
||||
// test (and the afterAll cleanup) at the jest 5s cap. With Date-only faking the
|
||||
// query resolves normally, and we still prove the stamp is the DB clock (not the
|
||||
// skewed process clock).
|
||||
function fakeDateOnly(iso: string): void {
|
||||
jest.useFakeTimers({
|
||||
doNotFake: [
|
||||
'hrtime',
|
||||
'nextTick',
|
||||
'performance',
|
||||
'queueMicrotask',
|
||||
'requestAnimationFrame',
|
||||
'cancelAnimationFrame',
|
||||
'requestIdleCallback',
|
||||
'cancelIdleCallback',
|
||||
'setImmediate',
|
||||
'clearImmediate',
|
||||
'setInterval',
|
||||
'clearInterval',
|
||||
'setTimeout',
|
||||
'clearTimeout',
|
||||
],
|
||||
now: new Date(iso),
|
||||
});
|
||||
}
|
||||
|
||||
const maybe = (name: string, fn: () => Promise<void>) =>
|
||||
it(name, async () => {
|
||||
if (!reachable) {
|
||||
@@ -188,9 +234,10 @@ describe('AiChatMessageRepo.findByChatUpdatedAfter (#491 delta poll)', () => {
|
||||
'update() stamps updatedAt from the DB clock, not the app clock',
|
||||
async () => {
|
||||
const m = await seedMessage();
|
||||
// Fake the PROCESS clock ~73 years into the future. If the stamp came from
|
||||
// `new Date()` the row would read year 2099; sql now() keeps it on DB time.
|
||||
jest.useFakeTimers().setSystemTime(new Date('2099-01-01T00:00:00Z'));
|
||||
// Skew the PROCESS clock ~73 years into the future (Date only). If the stamp
|
||||
// came from `new Date()` the row would read year 2099; sql now() keeps it on
|
||||
// DB time.
|
||||
fakeDateOnly('2099-01-01T00:00:00Z');
|
||||
const updated = await msgRepo.update(m.id, workspaceId, {
|
||||
content: 'y',
|
||||
});
|
||||
@@ -211,7 +258,7 @@ describe('AiChatMessageRepo.findByChatUpdatedAfter (#491 delta poll)', () => {
|
||||
status: 'running',
|
||||
stepCount: 0,
|
||||
} as never);
|
||||
jest.useFakeTimers().setSystemTime(new Date('2099-01-01T00:00:00Z'));
|
||||
fakeDateOnly('2099-01-01T00:00:00Z');
|
||||
const updated = await runRepo.update(run.id, workspaceId, {
|
||||
stepCount: 1,
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user