diff --git a/docs/de/platform/projects/concepts.md b/docs/de/platform/projects/concepts.md index 2e3aeb407d..0dcdada346 100644 --- a/docs/de/platform/projects/concepts.md +++ b/docs/de/platform/projects/concepts.md @@ -13,7 +13,7 @@ Ein Projekt ist die Einheit, zu der Tale greift, wenn ein Arbeitsvorhaben diesel ## Was ein Projekt besitzt -**Chats**, die im Projekt gestartet werden, tragen seinen Kontext automatisch. Sie bleiben deine, bis du an einem Chat **Mit Projekt teilen** umlegst — der Chats-Tab teilt sich entsprechend in **Deine Chats** und **Mit Projekt geteilt**. Das Teilen eines Chats blendet deine persönlichen Erinnerungen und Anweisungen aus den Antworten aus, die andere Mitglieder sehen. +**Chats**, die im Projekt gestartet werden, tragen seinen Kontext automatisch. Sie bleiben deine, bis du an einem Chat **Mit Projekt teilen** umlegst — der Chats-Tab teilt sich entsprechend in **Deine Chats** und **Mit Projekt geteilt**. Das Teilen eines Chats blendet deine persönlichen Erinnerungen und Anweisungen aus den Antworten aus, die andere Mitglieder sehen. Verschiebst du einen geteilten Chat in ein anderes Projekt — oder nimmst ihn aus seinem Projekt heraus —, endet das Teilen, damit ein neues Publikum ihn nicht stillschweigend erbt: Lege **Mit Projekt teilen** wieder um, wenn die Mitglieder des neuen Projekts ihn lesen sollen. **Anweisungen** sind Kontext, der für jeden Chat im Projekt gilt — die Rahmung, die Randbedingungen und das Vokabular der Arbeit —, damit niemand sie pro Chat neu einfügt. diff --git a/docs/en/platform/projects/concepts.md b/docs/en/platform/projects/concepts.md index 52f1e47b37..05132ffdbd 100644 --- a/docs/en/platform/projects/concepts.md +++ b/docs/en/platform/projects/concepts.md @@ -13,7 +13,7 @@ A project is the unit Tale reaches for when a body of work needs the same files, ## What a project owns -**Chats** started inside the project carry its context automatically. They stay yours until you flip **Share with project** on a chat — the Chats tab splits into **Your chats** and **Shared with project** accordingly. Sharing a chat hides your personal memories and instructions from the responses other members see. +**Chats** started inside the project carry its context automatically. They stay yours until you flip **Share with project** on a chat — the Chats tab splits into **Your chats** and **Shared with project** accordingly. Sharing a chat hides your personal memories and instructions from the responses other members see. Moving a shared chat to another project — or out of its project — ends the share, so a new audience never inherits it silently: switch **Share with project** back on if the new project's members should read it. **Instructions** are context that applies to every chat in the project — the framing, constraints, and vocabulary of the work — so nobody re-pastes them per chat. diff --git a/docs/fr/platform/projects/concepts.md b/docs/fr/platform/projects/concepts.md index f733be11c3..170e3caa9c 100644 --- a/docs/fr/platform/projects/concepts.md +++ b/docs/fr/platform/projects/concepts.md @@ -13,7 +13,7 @@ Un projet est l’unité que Tale sort quand un chantier a besoin des mêmes fic ## Ce qu’un projet possède -Les **chats** démarrés dans le projet portent son contexte automatiquement. Ils restent les tiens jusqu’à ce que tu actives **Partager avec le projet** sur un chat — l’onglet Chats se divise en **Tes chats** et **Partagés avec le projet** en conséquence. Partager un chat masque tes souvenirs et tes instructions personnels dans les réponses que voient les autres membres. +Les **chats** démarrés dans le projet portent son contexte automatiquement. Ils restent les tiens jusqu’à ce que tu actives **Partager avec le projet** sur un chat — l’onglet Chats se divise en **Tes chats** et **Partagés avec le projet** en conséquence. Partager un chat masque tes souvenirs et tes instructions personnels dans les réponses que voient les autres membres. Déplacer un chat partagé vers un autre projet — ou le sortir de son projet — met fin au partage, pour qu’un nouveau public n’en hérite jamais en silence : réactive **Partager avec le projet** si les membres du nouveau projet doivent le lire. Les **instructions** sont du contexte qui s’applique à chaque chat du projet — le cadre, les contraintes et le vocabulaire du travail — pour que personne ne les recolle chat par chat. diff --git a/services/platform/backend/core/chat/stream_stall.test.ts b/services/platform/backend/core/chat/stream_stall.test.ts new file mode 100644 index 0000000000..1a8a3260ba --- /dev/null +++ b/services/platform/backend/core/chat/stream_stall.test.ts @@ -0,0 +1,76 @@ +// @vitest-environment node + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { + createStallGuard, + STREAM_STALL_TIMEOUT_MS, + stallMessage, +} from './stream_stall'; + +/** + * The stall guard is a SILENCE clock, not a deadline: activity restarts it, + * so a stream that keeps producing can run for any length of time, and only + * a provider that goes quiet for the whole window trips it. + */ + +beforeEach(() => { + vi.useFakeTimers(); +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +describe('createStallGuard', () => { + it('never fires while activity keeps arriving, however long the stream runs', () => { + const guard = createStallGuard(1_000); + // Ten windows' worth of wall clock, touched just before each deadline. + for (let i = 0; i < 10; i++) { + vi.advanceTimersByTime(999); + guard.touch(); + } + expect(guard.signal.aborted).toBe(false); + expect(guard.stalled).toBe(false); + guard.dispose(); + }); + + it('fires once the provider has been silent for the whole window', () => { + const guard = createStallGuard(1_000); + vi.advanceTimersByTime(999); + expect(guard.signal.aborted).toBe(false); + vi.advanceTimersByTime(1); + expect(guard.signal.aborted).toBe(true); + expect(guard.stalled).toBe(true); + expect(guard.signal.reason).toBeInstanceOf(Error); + expect((guard.signal.reason as Error).message).toBe(stallMessage(1_000)); + }); + + it('measures silence from the LAST byte, not from the request start', () => { + const guard = createStallGuard(1_000); + vi.advanceTimersByTime(800); + guard.touch(); + // 1.6s since the start — past a fixed deadline, but only 0.8s of silence. + vi.advanceTimersByTime(800); + expect(guard.signal.aborted).toBe(false); + vi.advanceTimersByTime(200); + expect(guard.signal.aborted).toBe(true); + }); + + it('dispose stops the clock, and a late touch does not re-arm it', () => { + const guard = createStallGuard(1_000); + guard.dispose(); + guard.touch(); + vi.advanceTimersByTime(5_000); + expect(guard.signal.aborted).toBe(false); + expect(guard.stalled).toBe(false); + }); + + it('names the silence window and the timeout in the surfaced error', () => { + const guard = createStallGuard(STREAM_STALL_TIMEOUT_MS); + const error = guard.error(new Error('aborted')); + expect(error.message).toMatch(/timed out after 180 seconds of silence/); + expect(error.cause).toBeInstanceOf(Error); + guard.dispose(); + }); +}); diff --git a/services/platform/backend/core/chat/stream_stall.ts b/services/platform/backend/core/chat/stream_stall.ts new file mode 100644 index 0000000000..8d871ad5fc --- /dev/null +++ b/services/platform/backend/core/chat/stream_stall.ts @@ -0,0 +1,79 @@ +/** + * The silence clock for a streaming model round. + * + * A model round used to run under ONE fixed wall-clock abort on the fetch — + * a cap that could not tell a reply still streaming healthily at minute four + * from a connection that died at minute one, so every long reply (a high + * reasoning effort, a large output ceiling) was cut mid-sentence at exactly + * the deadline and surfaced as a generic provider error. This guard measures + * SILENCE instead: its clock restarts on every byte the provider sends, so a + * stream that keeps producing is never aborted however long it runs, and only + * a provider that stops sending for the whole window ends the round. The + * first byte gets the same allowance — a slow-thinking model is not a hung + * one. + */ + +/** How long the provider may stay silent — measured BETWEEN bytes, never + * from the request start — before the round is abandoned as stalled. */ +export const STREAM_STALL_TIMEOUT_MS = 180_000; + +export interface StallGuard { + /** Aborts once the provider has been silent for the whole window. Attach + * it to the fetch alongside the turn's own cancel signal. */ + readonly signal: AbortSignal; + /** True once THIS guard aborted the signal — a stall, as opposed to a user + * cancel riding the same fetch. */ + readonly stalled: boolean; + /** Bytes arrived: restart the silence clock. */ + touch(): void; + /** The round ended (either way): stop the clock so nothing fires late. */ + dispose(): void; + /** The failure to surface for a stall, in the user's face. "timed out" + * lands it in the chat-error classifier's transient bucket. */ + error(cause?: unknown): Error; +} + +export function stallMessage(timeoutMs: number): string { + return `The model provider stopped sending data — the reply timed out after ${Math.round(timeoutMs / 1000)} seconds of silence.`; +} + +export function createStallGuard( + timeoutMs: number = STREAM_STALL_TIMEOUT_MS, +): StallGuard { + const controller = new AbortController(); + let timer: ReturnType | undefined; + let stalled = false; + let disposed = false; + const clear = (): void => { + if (timer !== undefined) clearTimeout(timer); + timer = undefined; + }; + const error = (cause?: unknown): Error => + new Error( + stallMessage(timeoutMs), + cause === undefined ? undefined : { cause }, + ); + const arm = (): void => { + clear(); + timer = setTimeout(() => { + timer = undefined; + stalled = true; + controller.abort(error()); + }, timeoutMs); + }; + arm(); + return { + signal: controller.signal, + get stalled() { + return stalled; + }, + touch() { + if (!disposed && !stalled) arm(); + }, + dispose() { + disposed = true; + clear(); + }, + error, + }; +} diff --git a/services/platform/backend/core/chat/turn_action.test.ts b/services/platform/backend/core/chat/turn_action.test.ts new file mode 100644 index 0000000000..4e030f3f5e --- /dev/null +++ b/services/platform/backend/core/chat/turn_action.test.ts @@ -0,0 +1,91 @@ +// @vitest-environment node + +import { describe, expect, it } from 'vitest'; + +import { createStallGuard, type StallGuard } from './stream_stall'; +import { streamSse } from './turn_action'; + +/** + * The provider stream's one clock is a silence clock: a reply that keeps + * arriving is never cut, however long it runs past what a fixed deadline + * would have allowed, and only a provider that stops sending ends the round + * — with a failure that names the stall, not a generic abort. + */ + +function frame(text: string): string { + return `data: ${JSON.stringify({ + choices: [{ index: 0, delta: { content: text }, finish_reason: null }], + })}\n\n`; +} + +/** An SSE body that emits `count` frames `everyMs` apart and then closes — + * or, with `hang`, goes silent forever after the last one. */ +function drippingResponse(options: { + count: number; + everyMs: number; + hang?: boolean; +}): Response { + const encoder = new TextEncoder(); + const stream = new ReadableStream({ + start(controller) { + let sent = 0; + const tick = (): void => { + sent += 1; + controller.enqueue(encoder.encode(frame(`tick${sent} `))); + if (sent < options.count) setTimeout(tick, options.everyMs); + else if (options.hang !== true) controller.close(); + }; + setTimeout(tick, options.everyMs); + }, + }); + return new Response(stream, { + headers: { 'content-type': 'text/event-stream' }, + }); +} + +async function collect(response: Response, guard: StallGuard): Promise { + const texts: string[] = []; + for await (const chunk of streamSse(response, 'openai', guard)) { + texts.push(chunk.text); + } + return texts.join(''); +} + +describe('streamSse under the stall guard', () => { + it('keeps a healthy stream alive far past the silence window', async () => { + const guard = createStallGuard(150); + // 30 frames 15ms apart: ~450ms of streaming against a 150ms window — a + // fixed deadline of the window's length would have cut this reply at + // frame ten. + const text = await collect( + drippingResponse({ count: 30, everyMs: 15 }), + guard, + ); + guard.dispose(); + expect(text.startsWith('tick1 tick2 ')).toBe(true); + expect(text.endsWith('tick30 ')).toBe(true); + expect(guard.stalled).toBe(false); + }); + + it('ends a stream whose provider goes silent, naming the stall', async () => { + const guard = createStallGuard(100); + await expect( + collect(drippingResponse({ count: 2, everyMs: 10, hang: true }), guard), + ).rejects.toThrow(/timed out after \d+ seconds of silence/); + guard.dispose(); + expect(guard.stalled).toBe(true); + }); + + it('lets a user cancel riding the same fetch through as itself, not as a stall', async () => { + const guard = createStallGuard(1_000); + const abort = new DOMException('The operation was aborted.', 'AbortError'); + const stream = new ReadableStream({ + start(controller) { + setTimeout(() => controller.error(abort), 5); + }, + }); + await expect(collect(new Response(stream), guard)).rejects.toBe(abort); + guard.dispose(); + expect(guard.stalled).toBe(false); + }); +}); diff --git a/services/platform/backend/core/chat/turn_action.ts b/services/platform/backend/core/chat/turn_action.ts index 387fd292b0..3fac044d84 100644 --- a/services/platform/backend/core/chat/turn_action.ts +++ b/services/platform/backend/core/chat/turn_action.ts @@ -72,9 +72,9 @@ import { sanitizeError } from '../lib/utils/sanitize_secrets'; import { resolveProviderCredential } from '../provider_credentials/resolve_credential'; import { createChatToolExecutor } from './assistant_tools'; import { resolveProjectContext } from './project_context'; +import { createStallGuard, type StallGuard } from './stream_stall'; import { createConvexTurnStore, createConvexUsageLedger } from './turn_store'; -const REQUEST_TIMEOUT_MS = 180_000; /** The stored excerpt of an upstream error body. This is the ONLY record of * the provider's answer anywhere (nothing logs the full body), so it must fit * a whole pretty-printed provider error — secrets are handled by redaction @@ -407,12 +407,35 @@ function asRecord(value: unknown): Record | null { return isRecord(value) ? value : null; } +/** A promise that rejects with the signal's reason once it aborts — the + * stall clock's side of the per-read race below. Marked handled here, so a + * stream that ends before the clock fires never leaves an unhandled + * rejection behind. */ +function rejectOnAbort(signal: AbortSignal): Promise { + const rejection = new Promise((_, reject) => { + const fail = (): void => { + reject( + signal.reason instanceof Error + ? signal.reason + : new Error(String(signal.reason)), + ); + }; + if (signal.aborted) fail(); + else signal.addEventListener('abort', fail, { once: true }); + }); + rejection.catch(() => undefined); + return rejection; +} + /** Read a provider's Server-Sent Events stream line by line, yielding each * `data:` payload as a chunk of cleared text (and the final usage when it - * arrives). */ -async function* streamSse( + * arrives). With a stall guard, every byte restarts its silence clock and + * every read races it, so a provider that stops sending ends the round with + * the guard's error even where the runtime would leave the read pending. */ +export async function* streamSse( response: Response, apiFormat: ApiFormat, + stall?: StallGuard, ): AsyncGenerator { const body = response.body; if (!body) throw new Error('the model returned no response body to stream'); @@ -424,10 +447,34 @@ async function* streamSse( }; let buffer = ''; let lastUsage: TurnUsage | undefined; + const stalled = stall === undefined ? undefined : rejectOnAbort(stall.signal); while (true) { - const { done, value } = await reader.read(); + const pending = reader.read(); + let next: Awaited; + if (stalled === undefined) { + next = await pending; + } else { + try { + next = await Promise.race([pending, stalled]); + } catch (error) { + // The abandoned read settles later (the aborted fetch fails it) and + // must not surface as an unhandled rejection; the reader itself is + // released so the connection does not linger. + void pending.then( + () => undefined, + () => undefined, + ); + void reader.cancel().then( + () => undefined, + () => undefined, + ); + throw stall?.stalled === true ? stall.error(error) : error; + } + } + const { done, value } = next; if (done) break; + stall?.touch(); buffer += decoder.decode(value, { stream: true }); let newline = buffer.indexOf('\n'); while (newline !== -1) { @@ -645,10 +692,16 @@ export function createDirectModelCall( : { stream_options: { include_usage: true } }), }); - const timeout = AbortSignal.timeout(REQUEST_TIMEOUT_MS); + // The round's only clock is a SILENCE clock — never a whole-request + // deadline. A reply that keeps streaming is never cut, however long it + // runs (a high reasoning effort with a large output ceiling routinely + // passes three minutes while perfectly healthy); only a provider that + // stops sending for the whole window ends the round. The first byte + // gets the same allowance: a slow-thinking model is not a hung one. + const stall = createStallGuard(); const signal = request.signal - ? AbortSignal.any([request.signal, timeout]) - : timeout; + ? AbortSignal.any([request.signal, stall.signal]) + : stall.signal; let response: Response; try { @@ -659,6 +712,8 @@ export function createDirectModelCall( signal, }); } catch (error) { + stall.dispose(); + if (stall.stalled) throw stall.error(error); throw new Error( `The model provider was unreachable: ${sanitizeError(error, ERROR_EXCERPT)}`, { cause: error }, @@ -666,6 +721,7 @@ export function createDirectModelCall( } if (!response.ok) { const detail = await response.text().catch(() => ''); + stall.dispose(); // The HTTP status rides on the error so the chat-error classifier can // bucket it precisely (401/402/429…) instead of regexing the text. throw Object.assign( @@ -675,7 +731,13 @@ export function createDirectModelCall( { status: response.status }, ); } - yield* streamSse(response, wire.apiFormat); + // Headers count as the first sign of life; the body's bytes take over. + stall.touch(); + try { + yield* streamSse(response, wire.apiFormat, stall); + } finally { + stall.dispose(); + } }; } diff --git a/services/platform/backend/domains/chat/arena.test.ts b/services/platform/backend/domains/chat/arena.test.ts new file mode 100644 index 0000000000..7f52a8d478 --- /dev/null +++ b/services/platform/backend/domains/chat/arena.test.ts @@ -0,0 +1,144 @@ +// @vitest-environment node + +/** + * Arena's two columns must be the SAME conversation under two models: column + * B is born with A's project filing (so both turns get the project's + * instructions and knowledge) and a winning B keeps what the conversation + * had on A. The real-Postgres probe rides `integration-check.ts`; this locks + * the statements. + */ + +import type { Sql } from 'postgres'; +import { describe, expect, it } from 'vitest'; + +import { ensureArenaPair, settleArenaPair } from './arena.ts'; + +interface Statement { + text: string; + values: unknown[]; +} + +const THREAD_A = { + id: 'thread_a', + organizationId: 'org_1', + userId: 'user_1', + title: 'Pricing question', + kind: 'chat', + agentSlug: 'assistant', + harness: null, + capabilities: { skills: ['docx'], connectors: [] }, + reasoningEffort: 'high', + projectId: 'project_1', + sharedWithProject: false, + archived: false, + pinnedAt: 5_000, + lastReplyAt: null, + lastReadAt: 6_000, + isShared: false, + shareToken: null, + sharedAt: null, + sharedBy: null, + status: 'active', + branchRootId: null, + hidden: null, + createdAt: 1, + updatedAt: 1, +}; + +function fakeSql(answer: (statement: Statement) => unknown[] | undefined): { + sql: Sql; + statements: Statement[]; +} { + const statements: Statement[] = []; + const tag = (strings: TemplateStringsArray, ...values: unknown[]) => { + const statement = { text: strings.join('?'), values }; + statements.push(statement); + return Promise.resolve(answer(statement) ?? []); + }; + tag.unsafe = (text: string) => text; + tag.json = (value: unknown) => ({ json: value }); + tag.begin = (fn: (tx: unknown) => Promise) => fn(tag); + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- arena exercises exactly the tag, unsafe, json, and begin surfaces faked here + return { sql: tag as unknown as Sql, statements }; +} + +const ARGS = { + organizationId: 'org_1', + userId: 'user_1', + threadId: 'thread_a', +}; + +describe('ensureArenaPair', () => { + it("gives column B the conversation's project filing and effort pick", async () => { + const { sql, statements } = fakeSql((statement) => { + if (statement.text.includes('FROM app.threads t')) return [THREAD_A]; + if (statement.text.includes('SELECT arena FROM')) + return [{ arena: null }]; + if (statement.text.includes('INSERT INTO app.threads')) { + return [{ id: 'thread_b' }]; + } + return []; + }); + + await expect(ensureArenaPair(sql, ARGS)).resolves.toEqual({ + threadIdB: 'thread_b', + }); + + const birth = statements.find((s) => + s.text.includes('INSERT INTO app.thread_metadata'), + ); + expect(birth?.text).toContain('project_id'); + expect(birth?.text).toContain('reasoning_effort'); + expect(birth?.values).toContain('project_1'); + expect(birth?.values).toContain('high'); + // Still a hidden lineage sibling of A — never a second row in any list. + expect(birth?.values).toContain('thread_a'); + }); +}); + +describe('settleArenaPair', () => { + it("files a winning B where A was, with A's pin and read watermark", async () => { + const arenaOf = (threadId: unknown) => + threadId === 'thread_a' + ? { + pairId: 'pair', + role: 'a', + partnerThreadId: 'thread_b', + createdAt: 1, + } + : { + pairId: 'pair', + role: 'b', + partnerThreadId: 'thread_a', + createdAt: 1, + }; + const { sql, statements } = fakeSql((statement) => { + if (statement.text.includes('FROM app.threads t')) return [THREAD_A]; + if (statement.text.includes('SELECT arena FROM')) { + return [{ arena: arenaOf(statement.values[0]) }]; + } + return []; + }); + + await expect( + settleArenaPair(sql, { ...ARGS, verdict: 'b_better' }), + ).resolves.toEqual({ continueThreadId: 'thread_b' }); + + const graduation = statements.find( + (s) => + s.text.includes('UPDATE app.thread_metadata b') && + s.text.includes('hidden = NULL'), + ); + expect(graduation?.text).toContain( + 'project_id = coalesce(b.project_id, a.project_id)', + ); + expect(graduation?.text).toContain('pinned_at_ms = a.pinned_at_ms'); + expect(graduation?.text).toContain('last_read_at_ms = a.last_read_at_ms'); + expect(graduation?.values).toEqual([ + 'thread_b', + 'org_1', + 'thread_a', + 'org_1', + ]); + }); +}); diff --git a/services/platform/backend/domains/chat/arena.ts b/services/platform/backend/domains/chat/arena.ts index 513417999f..5c5b001165 100644 --- a/services/platform/backend/domains/chat/arena.ts +++ b/services/platform/backend/domains/chat/arena.ts @@ -153,9 +153,13 @@ export async function ensureArenaPair( const now = Date.now(); const pairId = mintPairId(); const threadIdB = await sql.begin(async (tx) => { - // B ties into A's lineage for the trash cascade but deliberately owns no - // project/share/voice state — the pair reads as ONE conversation and - // every outward-facing property stays on A. + // B ties into A's lineage for the trash cascade and carries A's project + // filing and effort pick: both columns must run with the same project + // instructions and knowledge — two prompts that differ compare prompts, + // not models — and a winning B keeps the conversation filed. Share and + // voice state stay on A: the pair reads as ONE conversation and its + // outward-facing properties live there (a hidden row never lists in the + // project's Chats tab). const inserted = await tx<{ id: string }[]>` INSERT INTO app.threads (org_id, user_id, title, kind, created_at_ms, updated_at_ms) @@ -168,11 +172,13 @@ export async function ensureArenaPair( await tx` INSERT INTO app.thread_metadata ( thread_id, org_id, user_id, chat_type, status, agent_slug, - capabilities, hidden, branch_root_id, archived, created_at_ms + capabilities, reasoning_effort, project_id, hidden, branch_root_id, + archived, created_at_ms ) VALUES ( ${idB}, ${args.organizationId}, ${args.userId}, ${thread.kind}, 'active', ${thread.agentSlug}, ${thread.capabilities === null ? null : tx.json(toJson(thread.capabilities))}, + ${thread.reasoningEffort}, ${thread.projectId}, true, ${thread.branchRootId ?? thread.id}, false, ${now} ) `; @@ -380,12 +386,19 @@ export async function settleArenaPair( WHERE thread_id = ${loserId} AND org_id = ${args.organizationId} `; if (winnerId === idB) { - // B graduates to a standalone visible conversation. The losing A stays - // a hidden root until retention reaps it. + // B graduates to a standalone visible conversation and takes over what + // the conversation had on A — its pin, its read watermark, and (for a + // pair opened before B carried the project) its filing. The losing A + // stays a hidden root until retention reaps it. await tx` - UPDATE app.thread_metadata - SET arena = NULL, hidden = NULL, branch_root_id = NULL - WHERE thread_id = ${idB} AND org_id = ${args.organizationId} + UPDATE app.thread_metadata b + SET arena = NULL, hidden = NULL, branch_root_id = NULL, + project_id = coalesce(b.project_id, a.project_id), + pinned_at_ms = a.pinned_at_ms, + last_read_at_ms = a.last_read_at_ms + FROM app.thread_metadata a + WHERE b.thread_id = ${idB} AND b.org_id = ${args.organizationId} + AND a.thread_id = ${idA} AND a.org_id = ${args.organizationId} `; } else { await tx` diff --git a/services/platform/backend/domains/chat/deferred-sends.ts b/services/platform/backend/domains/chat/deferred-sends.ts index efa4d2b1c4..16bdc1017d 100644 --- a/services/platform/backend/domains/chat/deferred-sends.ts +++ b/services/platform/backend/domains/chat/deferred-sends.ts @@ -1,5 +1,6 @@ import type { Sql } from 'postgres'; +import { ThreadBusyError } from '../../../lib/chat/turn.ts'; import { toJson } from '../../db/sql.ts'; import { addJobInTx } from '../../jobs/enqueue.ts'; import { isBackendDraining } from '../control/service.ts'; @@ -424,6 +425,7 @@ export async function pollDeferredSend( `; if (claimed.length === 0) return 'gone'; + let parkedAgain = false; try { // The claimed videos' transcripts join the send now (the 0.4 // `buildBoundJobAttachments` semantics — a job without a completed @@ -461,11 +463,29 @@ export async function pollDeferredSend( `[deferred-send] turn refused for ${row.id}: ${outcome.reason}`, ); } + } catch (error) { + // Lost the thread to a send that slipped in between the busy read above + // and the turn's atomic open. Nothing was appended, so this is the same + // "wait our turn" as the read — park the row again rather than drop the + // message into a deleted tray row. + if (error instanceof ThreadBusyError) { + await sql` + UPDATE app.deferred_sends + SET status = 'waiting', waiting_since_ms = ${Date.now()} + WHERE id = ${row.id} AND status = 'claimed' + `; + await reschedule(READY_POLL_MS); + parkedAgain = true; + return 'busy'; + } + throw error; } finally { // The terminal mop-up: the row settles whether the turn completed, // refused, or threw — the thread shows the bubble (or nothing), and the // tray row would only double-display or wedge. - await sql`DELETE FROM app.deferred_sends WHERE id = ${row.id}`; + if (!parkedAgain) { + await sql`DELETE FROM app.deferred_sends WHERE id = ${row.id}`; + } } return 'ran'; } diff --git a/services/platform/backend/domains/chat/rest-turn.ts b/services/platform/backend/domains/chat/rest-turn.ts index d950bc97da..1b02ff0947 100644 --- a/services/platform/backend/domains/chat/rest-turn.ts +++ b/services/platform/backend/domains/chat/rest-turn.ts @@ -87,6 +87,10 @@ export async function runApiTurn( ); } } catch (error) { + // A ThreadBusyError here is the busy gate above lost to a send that + // slipped in between the read and the turn's atomic open — the same + // fact, answered the same way: the caller sees why their message never + // got a reply. (The open rolled back; the other turn is untouched.) const reason = error instanceof Error ? error.message : 'The turn could not be started.'; console.warn(`[rest-turn] turn threw for ${payload.threadId}: ${reason}`); diff --git a/services/platform/backend/domains/chat/routes.ts b/services/platform/backend/domains/chat/routes.ts index 23cccccce4..8bf5f93759 100644 --- a/services/platform/backend/domains/chat/routes.ts +++ b/services/platform/backend/domains/chat/routes.ts @@ -3,6 +3,7 @@ import { streamSSE } from 'hono/streaming'; import type { Sql } from 'postgres'; import { z } from 'zod'; +import { THREAD_BUSY_REASON, ThreadBusyError } from '../../../lib/chat/turn.ts'; import { classifyChatErrorCode, encodeChatError, @@ -503,8 +504,11 @@ export function createChatRoutes(deps: { sql: Sql; auth: Auth }): Hono { try { const ok = await moveThreadToProject( deps.sql, - organizationId, - userId, + { + organizationId, + userId, + email: c.get('sessionBundle').user.email, + }, c.req.param('threadId'), body.data.projectId, ); @@ -623,16 +627,18 @@ export function createChatRoutes(deps: { sql: Sql; auth: Auth }): Hono { return c.json(share); }); + // Works on a trashed thread too — revoking a link must never depend on + // the conversation being visible. app.post('/threads/:threadId/unshare', async (c) => { const { organizationId, userId } = caller(c); - await unshareThread( + const ok = await unshareThread( deps.sql, organizationId, userId, c.req.param('threadId'), ); - await hintThread(c, c.req.param('threadId')); - return c.json({ ok: true }); + if (ok) await hintThread(c, c.req.param('threadId')); + return c.json({ ok }); }); app.post('/threads/:threadId/branch', async (c) => { @@ -1152,10 +1158,7 @@ export function createChatRoutes(deps: { sql: Sql; auth: Auth }): Hono { (await hasLiveGeneration(deps.sql, organizationId, pair.threadIdA)) || (await hasLiveGeneration(deps.sql, organizationId, pair.threadIdB)) ) { - const busy = { - status: 'refused' as const, - reason: 'This conversation is already generating a response.', - }; + const busy = { status: 'refused' as const, reason: THREAD_BUSY_REASON }; return c.json({ a: busy, b: busy }); } @@ -1191,6 +1194,12 @@ export function createChatRoutes(deps: { sql: Sql; auth: Auth }): Hono { : {}), }; } catch (err) { + // Lost the column's claim to a send that slipped past the busy read + // above: the open rolled back and the other turn owns the thread — + // an error row now would land in ITS transcript. + if (err instanceof ThreadBusyError) { + return { status: 'refused', reason: err.message }; + } // A pre-pipeline throw (model resolution, credential) left nothing // in the transcript — write the error row here so the column // explains itself instead of sitting silently half-empty. @@ -1260,17 +1269,14 @@ export function createChatRoutes(deps: { sql: Sql; auth: Auth }): Hono { 503, ); } - // At most one turn per thread — refuse a concurrent send rather than - // let two turns interleave and delete each other's generation row. + // At most one turn per thread. This read is the fast path that spares a + // busy thread the model resolution; the GUARD is the turn's own atomic + // open (`beginTurn` claims the generation row and rejects a loser with + // ThreadBusyError), so two sends racing through this check cannot both + // run and delete each other's row. const live = await readGeneration(deps.sql, organizationId, thread.id); if (live !== null) { - return c.json( - { - status: 'refused', - reason: 'This conversation is already generating a response.', - }, - 409, - ); + return c.json({ status: 'refused', reason: THREAD_BUSY_REASON }, 409); } let outcome; try { @@ -1298,6 +1304,11 @@ export function createChatRoutes(deps: { sql: Sql; auth: Auth }): Hono { ...(body.data.resend === true ? { resend: true } : {}), }); } catch (error) { + // The claim loser of two racing sends: nothing was appended, the + // other turn streams on — the same refusal the fast path gives. + if (error instanceof ThreadBusyError) { + return c.json({ status: 'refused', reason: error.message }, 409); + } // A turn that could not START because the picked model is not // servable — its provider's default credential was disabled or // deleted, or the composer still holds a model the picker has since diff --git a/services/platform/backend/domains/chat/shim.test.ts b/services/platform/backend/domains/chat/shim.test.ts index 59bd560cd9..f440fadff7 100644 --- a/services/platform/backend/domains/chat/shim.test.ts +++ b/services/platform/backend/domains/chat/shim.test.ts @@ -2,6 +2,7 @@ import { readFileSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; +import type { Sql } from 'postgres'; import { describe, expect, it } from 'vitest'; import { @@ -94,3 +95,32 @@ describe('chatShimHandlers', () => { ); }); }); + +/** + * The chat shim's entity reads must apply the same lifecycle rules the owning + * domains do — a record the user deleted is not "current" just because the + * assistant found it through a different door. + */ +function capturingSql(): { sql: Sql; texts: string[] } { + const texts: string[] = []; + const tag = (strings: TemplateStringsArray) => { + texts.push(strings.join('?')); + return Promise.resolve([]); + }; + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- only the tag call is exercised by the contact query + return { sql: tag as unknown as Sql, texts }; +} + +describe("chat shim 'contacts/internal_queries:queryContacts'", () => { + it('hides trashed contacts, like every read in the contacts domain', async () => { + const { sql, texts } = capturingSql(); + const handlers = chatShimHandlers(sql); + const query = handlers['contacts/internal_queries:queryContacts']; + if (query === undefined) throw new Error('contact query handler missing'); + + await query({ organizationId: 'org_1', searchTerm: 'ada' }); + + const contacts = texts.find((text) => text.includes('FROM app.contacts')); + expect(contacts).toContain("lifecycle_status IS DISTINCT FROM 'trashed'"); + }); +}); diff --git a/services/platform/backend/domains/chat/shim.ts b/services/platform/backend/domains/chat/shim.ts index 9ec1dc6334..c9cc3b9af9 100644 --- a/services/platform/backend/domains/chat/shim.ts +++ b/services/platform/backend/domains/chat/shim.ts @@ -922,6 +922,9 @@ export function chatShimHandlers(sql: Sql): ShimHandlers { lifecycle_status AS "lifecycleStatus" FROM app.contacts WHERE org_id = ${args.organizationId} + -- The contacts domain hides trashed rows from every read; a + -- contact the user deleted must not resurface in a chat answer. + AND lifecycle_status IS DISTINCT FROM 'trashed' AND (${term === ''} OR name ILIKE ${like} OR email ILIKE ${like} OR phone ILIKE ${like} OR (${words.length > 0} diff --git a/services/platform/backend/domains/chat/store.test.ts b/services/platform/backend/domains/chat/store.test.ts index 6637c09204..dba8e6e2d1 100644 --- a/services/platform/backend/domains/chat/store.test.ts +++ b/services/platform/backend/domains/chat/store.test.ts @@ -25,7 +25,12 @@ vi.mock('../../core/lib/providers/catalog_fetch.ts', () => ({ })); vi.mock('../../jobs/enqueue.ts', () => ({ addJobInTx: vi.fn() })); -import { createPgUsageLedger, estimateTurnCostCents } from './store.ts'; +import { ThreadBusyError } from '../../../lib/chat/turn.ts'; +import { + createPgTurnStore, + createPgUsageLedger, + estimateTurnCostCents, +} from './store.ts'; const OPENROUTER = { name: 'openrouter', @@ -109,3 +114,148 @@ describe('createPgUsageLedger', () => { } }); }); + +interface Statement { + text: string; + values: unknown[]; +} + +/** + * A transaction-aware fake `sql`: the pool tag and each `begin` callback's + * tag log to SEPARATE ledgers, so a test can prove which writes rode the + * transaction and which bypassed it. Statements are answered by shape — a + * message insert returns the next row id, the generations claim returns a + * row unless the fake is told the thread is held. + */ +function fakeChatSql(options: { threadHeld?: boolean } = {}): { + sql: Sql; + pool: Statement[]; + tx: Statement[]; + transactions: Array<'commit' | 'rollback'>; + notified: string[]; +} { + const pool: Statement[] = []; + const tx: Statement[] = []; + const transactions: Array<'commit' | 'rollback'> = []; + const notified: string[] = []; + let messageRows = 0; + const answer = (text: string): unknown[] => { + if (text.includes('INSERT INTO app.messages')) { + messageRows += 1; + return [{ id: `msg_${messageRows}`, order: messageRows - 1 }]; + } + if (text.includes('FROM app.thread_metadata WHERE thread_id')) { + return [{ branchRootId: null, chatType: 'chat', userId: 'user_1' }]; + } + if (text.includes('INSERT INTO app.generations')) { + return options.threadHeld === true ? [] : [{ threadId: 'thread_1' }]; + } + return []; + }; + const makeTag = (log: Statement[]) => { + const tag = (strings: TemplateStringsArray, ...values: unknown[]) => { + const text = strings.join('?'); + log.push({ text, values }); + return Promise.resolve(answer(text)); + }; + tag.json = (value: unknown) => ({ json: value }); + return tag; + }; + const pooled = Object.assign(makeTag(pool), { + notify(channel: string, payload: string) { + notified.push(`${channel}:${payload}`); + return Promise.resolve(); + }, + async begin(fn: (tx: unknown) => Promise) { + try { + const result = await fn(makeTag(tx)); + transactions.push('commit'); + return result; + } catch (error) { + transactions.push('rollback'); + throw error; + } + }, + }); + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- the turn store exercises exactly the tag, json, notify, and begin surfaces faked here + return { sql: pooled as unknown as Sql, pool, tx, transactions, notified }; +} + +const OPEN = { + organizationId: 'org_1', + threadId: 'thread_1', + userParts: [{ type: 'text' as const, text: 'hello' }], +}; + +describe('createPgTurnStore.beginTurn', () => { + it('opens the turn inside ONE transaction and notifies only once it committed', async () => { + const f = fakeChatSql(); + const opened = await createPgTurnStore(f.sql).beginTurn(OPEN); + + expect(opened.userMessage?.id).toBe('msg_1'); + expect(opened.assistantMessage.id).toBe('msg_2'); + expect(f.transactions).toEqual(['commit']); + // Every write rode the transaction; nothing touched the pool directly, + // so no crash between the three can leave a partial open behind. + expect(f.pool).toEqual([]); + const texts = f.tx.map((statement) => statement.text); + expect( + texts.filter((t) => t.includes('INSERT INTO app.messages')), + ).toHaveLength(2); + expect(texts.some((t) => t.includes('INSERT INTO app.generations'))).toBe( + true, + ); + expect( + texts.some((t) => t.includes("generation_status = 'generating'")), + ).toBe(true); + expect(f.notified).toEqual(['chat_stream:thread_1']); + }); + + it('claims the thread with DO NOTHING and rolls the whole open back when another turn holds it', async () => { + const f = fakeChatSql({ threadHeld: true }); + + await expect( + createPgTurnStore(f.sql).beginTurn(OPEN), + ).rejects.toBeInstanceOf(ThreadBusyError); + + expect(f.transactions).toEqual(['rollback']); + const claim = f.tx.find((statement) => + statement.text.includes('INSERT INTO app.generations'), + ); + expect(claim?.text).toContain('ON CONFLICT (thread_id) DO NOTHING'); + expect(claim?.text).not.toContain('DO UPDATE'); + // The loser announces nothing — no NOTIFY for a turn that never opened, + // and no sidecar write claiming the thread is generating. + expect(f.notified).toEqual([]); + expect( + f.tx.some((s) => s.text.includes("generation_status = 'generating'")), + ).toBe(false); + }); +}); + +describe('createPgTurnStore.endGeneration', () => { + it('closes the row, settles the sidecar, and fails a still-pending placeholder in one transaction', async () => { + const f = fakeChatSql(); + await createPgTurnStore(f.sql).endGeneration({ + organizationId: 'org_1', + threadId: 'thread_1', + }); + + expect(f.transactions).toEqual(['commit']); + expect(f.pool).toEqual([]); + const texts = f.tx.map((statement) => statement.text); + expect(texts.some((t) => t.includes('DELETE FROM app.generations'))).toBe( + true, + ); + expect(texts.some((t) => t.includes("generation_status = 'idle'"))).toBe( + true, + ); + expect( + texts.some( + (t) => + t.includes("status = 'failed'") && t.includes("status = 'pending'"), + ), + ).toBe(true); + expect(f.notified).toEqual(['chat_stream:thread_1']); + }); +}); diff --git a/services/platform/backend/domains/chat/store.ts b/services/platform/backend/domains/chat/store.ts index 536f760db0..ccc7f31c17 100644 --- a/services/platform/backend/domains/chat/store.ts +++ b/services/platform/backend/domains/chat/store.ts @@ -2,6 +2,7 @@ import type { Sql } from 'postgres'; import { estimateCostCents, + ThreadBusyError, type TurnStore, type UsageLedger, type UsageLedgerEntry, @@ -206,68 +207,95 @@ export function createPgTurnStore(sql: Sql): TurnStore { }, async beginTurn(setup) { - let userMessage: { id: string; sequence: number } | undefined; - if (setup.userParts !== undefined) { - userMessage = await appendMessageRow(sql, { + // ONE transaction, per the contract: the user row, the placeholder, + // and the generation row commit together or not at all — a crash + // between them used to leave a question with no reply, or a 'pending' + // bubble no watchdog would ever fail (the watchdog keys on the + // generation row, which did not exist yet). + const opened = await sql.begin(async (tx) => { + let userMessage: { id: string; sequence: number } | undefined; + if (setup.userParts !== undefined) { + userMessage = await appendMessageRow(tx, { + organizationId: setup.organizationId, + threadId: setup.threadId, + role: 'user', + parts: setup.userParts, + text: setup.userParts + .map((part) => (part.type === 'text' ? part.text : '')) + .join(''), + ...(setup.truncation !== undefined + ? { truncation: setup.truncation } + : {}), + }); + } + const assistantMessage = await appendMessageRow(tx, { organizationId: setup.organizationId, threadId: setup.threadId, - role: 'user', - parts: setup.userParts, - text: setup.userParts - .map((part) => (part.type === 'text' ? part.text : '')) - .join(''), - ...(setup.truncation !== undefined - ? { truncation: setup.truncation } - : {}), + role: 'assistant', + parts: [], + status: 'pending', }); - } - const assistantMessage = await appendMessageRow(sql, { - organizationId: setup.organizationId, - threadId: setup.threadId, - role: 'assistant', - parts: [], - status: 'pending', + const now = Date.now(); + // The claim. The row's existence is the at-most-one-turn fact every + // lane reads; a conflict means another turn holds the thread, and + // rebinding its row (the old DO UPDATE) let two racing sends stream + // into one row and delete it from under each other. DO NOTHING plus + // the throw rolls the whole open back — the loser leaves no trace. + const claimed = await tx<{ threadId: string }[]>` + INSERT INTO app.generations ( + thread_id, org_id, message_id, started_at_ms, heartbeat_at_ms, + updated_at_ms + ) VALUES ( + ${setup.threadId}, ${setup.organizationId}, ${assistantMessage.id}, + ${now}, ${now}, ${now} + ) + ON CONFLICT (thread_id) DO NOTHING + RETURNING thread_id AS "threadId" + `; + if (claimed.length === 0) throw new ThreadBusyError(setup.threadId); + await tx` + UPDATE app.thread_metadata SET + generation_status = 'generating', stream_id = ${assistantMessage.id}, + generation_start_ms = ${now}, generation_heartbeat_at_ms = ${now}, + cancelled_at_ms = NULL, cancelled_message_id = NULL + WHERE thread_id = ${setup.threadId} + `; + return { + ...(userMessage !== undefined ? { userMessage } : {}), + assistantMessage, + }; }); - const now = Date.now(); - await sql` - INSERT INTO app.generations ( - thread_id, org_id, message_id, started_at_ms, heartbeat_at_ms, - updated_at_ms - ) VALUES ( - ${setup.threadId}, ${setup.organizationId}, ${assistantMessage.id}, - ${now}, ${now}, ${now} - ) - ON CONFLICT (thread_id) DO UPDATE SET - message_id = ${assistantMessage.id}, text = '', reasoning = '', - cancel_requested = false, started_at_ms = ${now}, - heartbeat_at_ms = ${now}, updated_at_ms = ${now} - `; - await sql` - UPDATE app.thread_metadata SET - generation_status = 'generating', stream_id = ${assistantMessage.id}, - generation_start_ms = ${now}, generation_heartbeat_at_ms = ${now}, - cancelled_at_ms = NULL, cancelled_message_id = NULL - WHERE thread_id = ${setup.threadId} - `; await notifyThread(sql, setup.threadId); - return { - ...(userMessage !== undefined ? { userMessage } : {}), - assistantMessage, - }; + return opened; }, async endGeneration(generation) { - await sql` - DELETE FROM app.generations - WHERE thread_id = ${generation.threadId} - AND org_id = ${generation.organizationId} - `; - await sql` - UPDATE app.thread_metadata SET - generation_status = 'idle', stream_id = NULL, - generation_heartbeat_at_ms = NULL - WHERE thread_id = ${generation.threadId} - `; + await sql.begin(async (tx) => { + await tx` + DELETE FROM app.generations + WHERE thread_id = ${generation.threadId} + AND org_id = ${generation.organizationId} + `; + await tx` + UPDATE app.thread_metadata SET + generation_status = 'idle', stream_id = NULL, + generation_heartbeat_at_ms = NULL + WHERE thread_id = ${generation.threadId} + `; + // A settled turn leaves no 'pending' placeholder: the finalize write + // precedes this on every path (completed, refused, cancelled, paused, + // failed), so a row still pending here is one whose finalize itself + // failed — and with the generation row gone, nothing else would ever + // mark it. The claim above makes this turn's placeholder the only + // pending row the thread can hold. + await tx` + UPDATE app.messages SET status = 'failed', + error = coalesce(error, 'the turn ended before its reply settled') + WHERE thread_id = ${generation.threadId} + AND org_id = ${generation.organizationId} + AND status = 'pending' + `; + }); await notifyThread(sql, generation.threadId); }, }; diff --git a/services/platform/backend/domains/chat/threads.test.ts b/services/platform/backend/domains/chat/threads.test.ts new file mode 100644 index 0000000000..83f38bc37b --- /dev/null +++ b/services/platform/backend/domains/chat/threads.test.ts @@ -0,0 +1,203 @@ +// @vitest-environment node + +/** + * The sharing rules a conversation's lifecycle must honour: a share link + * serves nothing for a thread in the trash, revoking the link works there + * too, and refiling a project-shared thread never carries its audience into + * the new project. The real-Postgres probes ride `integration-check.ts`; + * these lock the statements the rules live in. + */ + +import type { Sql } from 'postgres'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { createAuditLog, findOrganizationMember, getUserTeamIds } = vi.hoisted( + () => ({ + createAuditLog: vi.fn(), + findOrganizationMember: vi.fn(), + getUserTeamIds: vi.fn(), + }), +); + +vi.mock('../audit_logs/service.ts', () => ({ createAuditLog })); +vi.mock('../../auth/membership.ts', () => ({ + findOrganizationMember, + getUserTeamIds, +})); + +import { + getSharedThread, + moveThreadToProject, + unshareThread, +} from './threads.ts'; + +interface Statement { + text: string; + values: unknown[]; +} + +const OWNED_ROW = { + id: 'thread_1', + organizationId: 'org_1', + userId: 'user_1', + title: 'Launch plan', + kind: 'chat', + agentSlug: null, + harness: null, + capabilities: null, + reasoningEffort: null, + projectId: 'project_a', + sharedWithProject: true, + archived: false, + pinnedAt: null, + lastReplyAt: null, + lastReadAt: null, + isShared: true, + shareToken: 'tok', + sharedAt: 1_000, + sharedBy: 'user_1', + status: 'active', + branchRootId: null, + hidden: null, + createdAt: 1, + updatedAt: 1, +}; + +/** A fake `sql` answering by statement shape; `answer` overrides per test. + * Pool and transaction statements land in one ledger, in order. */ +function fakeSql(answer: (statement: Statement) => unknown[] | undefined): { + sql: Sql; + statements: Statement[]; +} { + const statements: Statement[] = []; + const tag = (strings: TemplateStringsArray, ...values: unknown[]) => { + const statement = { text: strings.join('?'), values }; + statements.push(statement); + return Promise.resolve(answer(statement) ?? []); + }; + tag.unsafe = (text: string) => text; + tag.json = (value: unknown) => ({ json: value }); + tag.begin = (fn: (tx: unknown) => Promise) => fn(tag); + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- the thread functions exercise exactly the tag, unsafe, json, and begin surfaces faked here + return { sql: tag as unknown as Sql, statements }; +} + +beforeEach(() => { + vi.clearAllMocks(); + findOrganizationMember.mockResolvedValue({ role: 'owner' }); + getUserTeamIds.mockResolvedValue([]); +}); + +describe('getSharedThread', () => { + it('resolves the token only for an ACTIVE thread — trash and expiry go dark', async () => { + const { sql, statements } = fakeSql((statement) => + statement.text.includes('share_token') ? [OWNED_ROW] : [], + ); + const view = await getSharedThread(sql, ['org_1'], 'tok'); + + expect(view?.threadId).toBe('thread_1'); + const lookup = statements.find((s) => s.text.includes('share_token')); + expect(lookup?.text).toContain("tm.status = 'active'"); + }); +}); + +describe('unshareThread', () => { + it('revokes on the owner-matched row regardless of lifecycle, and says whether it did', async () => { + const { sql, statements } = fakeSql((statement) => + statement.text.includes('is_shared = false') + ? [{ threadId: 'thread_1' }] + : [], + ); + await expect( + unshareThread(sql, 'org_1', 'user_1', 'thread_1'), + ).resolves.toBe(true); + + // One statement — no active-thread read in front of the write, so a + // trashed thread's link is still revocable. + expect(statements).toHaveLength(1); + const revoke = statements[0]; + expect(revoke?.text).toContain('is_shared = false'); + expect(revoke?.text).not.toContain('status'); + expect(revoke?.values).toEqual(['thread_1', 'org_1', 'user_1']); + }); + + it('answers false for a thread the caller does not own', async () => { + const { sql } = fakeSql(() => []); + await expect( + unshareThread(sql, 'org_1', 'user_2', 'thread_1'), + ).resolves.toBe(false); + }); +}); + +describe('moveThreadToProject', () => { + const auth = { organizationId: 'org_1', userId: 'user_1', email: 'o@x.io' }; + const answering = + (row: typeof OWNED_ROW) => + (statement: Statement): unknown[] | undefined => { + if (statement.text.includes('FROM app.threads t')) return [row]; + if (statement.text.includes('FROM app.projects WHERE id')) { + return statement.text.includes('shared_with_team_ids') + ? [{ orgId: 'org_1', teamId: null, sharedWithTeamIds: [] }] + : [{ name: 'Project A' }]; + } + return []; + }; + + it('ends the project share when the thread changes project, and audits the old project', async () => { + const { sql, statements } = fakeSql(answering(OWNED_ROW)); + await expect( + moveThreadToProject(sql, auth, 'thread_1', 'project_b'), + ).resolves.toBe(true); + + const update = statements.find((s) => + s.text.includes('UPDATE app.thread_metadata'), + ); + expect(update?.text).toContain('shared_with_project = ?'); + expect(update?.values).toEqual(['project_b', false, 'thread_1']); + expect(createAuditLog).toHaveBeenCalledTimes(1); + expect(createAuditLog.mock.calls[0]?.[1]).toMatchObject({ + action: 'project.thread.unshared', + resourceType: 'project', + resourceId: 'project_a', + resourceName: 'Project A', + actorId: 'user_1', + actorEmail: 'o@x.io', + previousState: { threadId: 'thread_1', shared: true }, + newState: { + threadId: 'thread_1', + shared: false, + movedToProjectId: 'project_b', + }, + }); + }); + + it('ends the share when the thread is taken out of its project', async () => { + const { sql, statements } = fakeSql(answering(OWNED_ROW)); + await moveThreadToProject(sql, auth, 'thread_1', null); + + const update = statements.find((s) => + s.text.includes('UPDATE app.thread_metadata'), + ); + expect(update?.values).toEqual([null, false, 'thread_1']); + expect(createAuditLog).toHaveBeenCalledTimes(1); + }); + + it('keeps the share when the project does not change, and audits nothing', async () => { + const { sql, statements } = fakeSql(answering(OWNED_ROW)); + await moveThreadToProject(sql, auth, 'thread_1', 'project_a'); + + const update = statements.find((s) => + s.text.includes('UPDATE app.thread_metadata'), + ); + expect(update?.values).toEqual(['project_a', true, 'thread_1']); + expect(createAuditLog).not.toHaveBeenCalled(); + }); + + it('audits nothing for a thread that was never shared', async () => { + const { sql } = fakeSql( + answering({ ...OWNED_ROW, sharedWithProject: false }), + ); + await moveThreadToProject(sql, auth, 'thread_1', 'project_b'); + expect(createAuditLog).not.toHaveBeenCalled(); + }); +}); diff --git a/services/platform/backend/domains/chat/threads.ts b/services/platform/backend/domains/chat/threads.ts index 43121cf153..214e0bccfd 100644 --- a/services/platform/backend/domains/chat/threads.ts +++ b/services/platform/backend/domains/chat/threads.ts @@ -463,21 +463,32 @@ export async function setThreadReasoningEffort( return true; } -/** File a thread under a project, or take it back out (null). */ +/** + * File a thread under a project, or take it back out (null). Changing the + * project ENDS a project share: the owner's opt-in named one specific + * audience, and carrying the flag into another project would hand its + * members the whole history with no consent and no audit row. The implicit + * unshare is audited on the project the thread leaves — the owner re-shares + * in the new project deliberately. + */ export async function moveThreadToProject( sql: Sql, - organizationId: string, - userId: string, + auth: { organizationId: string; userId: string; email?: string }, threadId: string, projectId: string | null, ): Promise { - const thread = await loadOwnedThread(sql, organizationId, userId, threadId); + const thread = await loadOwnedThread( + sql, + auth.organizationId, + auth.userId, + threadId, + ); if (!thread) return false; if (projectId !== null) { const access = await projectChatAccess(sql, { projectId, - organizationId, - userId, + organizationId: auth.organizationId, + userId: auth.userId, }); if (access !== 'ok') { throw new ChatThreadError( @@ -487,10 +498,40 @@ export async function moveThreadToProject( ); } } - await sql` - UPDATE app.thread_metadata SET project_id = ${projectId} - WHERE thread_id = ${thread.id} - `; + const previousProjectId = thread.projectId; + const moved = projectId !== previousProjectId; + const endsShare = + moved && thread.sharedWithProject === true && previousProjectId !== null; + await sql.begin(async (tx) => { + await tx` + UPDATE app.thread_metadata SET + project_id = ${projectId}, + shared_with_project = ${moved ? false : thread.sharedWithProject} + WHERE thread_id = ${thread.id} + `; + if (!endsShare) return; + const projects = await tx<{ name: string }[]>` + SELECT name FROM app.projects WHERE id = ${previousProjectId} LIMIT 1 + `; + await createAuditLog(tx, { + organizationId: auth.organizationId, + actorId: auth.userId, + ...(auth.email !== undefined ? { actorEmail: auth.email } : {}), + actorType: 'user', + action: 'project.thread.unshared', + category: 'data', + resourceType: 'project', + resourceId: previousProjectId, + ...(projects[0] ? { resourceName: projects[0].name } : {}), + status: 'success', + previousState: { threadId: thread.id, shared: true }, + newState: { + threadId: thread.id, + shared: false, + movedToProjectId: projectId, + }, + }); + }); return true; } @@ -679,19 +720,28 @@ export async function shareThread( return { shareToken }; } -/** Stop sharing; the token is kept so re-sharing restores the same URL. */ +/** + * Stop sharing; the token is kept so re-sharing restores the same URL. + * Owner-matched on the row itself rather than through the active-thread + * read: a conversation already in the trash must stay revocable — the link's + * gate hides it meanwhile, but a restore would otherwise bring the share + * back without the owner ever having been able to switch it off. Returns + * whether an owned row was matched, so the route answers honestly. + */ export async function unshareThread( sql: Sql, organizationId: string, userId: string, threadId: string, -): Promise { - const thread = await loadOwnedThread(sql, organizationId, userId, threadId); - if (!thread) return; - await sql` - UPDATE app.thread_metadata SET is_shared = false - WHERE thread_id = ${thread.id} +): Promise { + const rows = await sql<{ threadId: string }[]>` + UPDATE app.thread_metadata tm SET is_shared = false + FROM app.threads t + WHERE tm.thread_id = t.id AND t.id = ${threadId} + AND t.org_id = ${organizationId} AND t.user_id = ${userId} + RETURNING tm.thread_id AS "threadId" `; + return rows.length > 0; } export async function getThreadShareStatus( @@ -738,8 +788,10 @@ export interface SharedThreadView { * Resolve a share token to its read-only snapshot. The token authorizes the * read TOGETHER with org membership (checked by the route's door — the org * is resolved FROM the thread here and compared). The snapshot is cut at - * `sharedAt`; unknown token, unshared thread, and cross-org caller are - * indistinguishable nulls by design. + * `sharedAt`; unknown token, unshared thread, trashed or aged-out thread, + * and cross-org caller are indistinguishable nulls by design — the + * lifecycle gate is the one every other read applies, so deleting a + * conversation revokes what its link serves. */ export async function getSharedThread( sql: Sql, @@ -750,7 +802,7 @@ export async function getSharedThread( SELECT ${sql.unsafe(THREAD_COLUMNS)} FROM app.threads t JOIN app.thread_metadata tm ON tm.thread_id = t.id - WHERE tm.share_token = ${shareToken} + WHERE tm.share_token = ${shareToken} AND tm.status = 'active' LIMIT 1 `; const thread = rows[0]; diff --git a/services/platform/backend/integration-check.ts b/services/platform/backend/integration-check.ts index 7d3939bffe..6ea25e1085 100644 --- a/services/platform/backend/integration-check.ts +++ b/services/platform/backend/integration-check.ts @@ -7633,6 +7633,73 @@ async function checkChat( tracedLive, `toolPartInProgressFrame=${tracedLive} (want true — not only in the settled payload)`, ); + + // Two sends racing into ONE thread (two tabs, a double-click): both can + // pass the busy read, so the turn's own atomic open must decide — + // exactly one runs to completion, the other is refused with 409, and the + // transcript gains one question and one FULL reply (the loser appended + // nothing and never closed the winner's generation row mid-drip). + const raceThread = z.object({ id: z.string() }).safeParse( + await ( + await send(`/api/app/chat/threads?orgId=${orgId}`, { + title: 'Itest racing sends', + }) + ).json(), + ); + const raceThreadId = raceThread.success ? raceThread.data.id : ''; + const raceBody = (tab: string): unknown => ({ + text: `${SLOW_MARKER} from ${tab}`, + modelId: 'itest-chat', + providerSlug: 'itestchat', + }); + const raced = await Promise.all([ + send( + `/api/app/chat/threads/${raceThreadId}/messages?orgId=${orgId}`, + raceBody('tab one'), + ), + send( + `/api/app/chat/threads/${raceThreadId}/messages?orgId=${orgId}`, + raceBody('tab two'), + ), + ]); + const raceOutcome = z.object({ status: z.string() }); + const raceStatuses = ( + await Promise.all( + raced.map(async (response) => { + const parsed = raceOutcome.safeParse(await response.json()); + return parsed.success ? parsed.data.status : 'ERR'; + }), + ) + ).sort((a, b) => a.localeCompare(b)); + const raceHttp = raced + .map((response) => response.status) + .sort((a, b) => a - b); + const raceRows = await sql< + { role: string; status: string; text: string | null }[] + >` + SELECT role, status, text FROM app.messages + WHERE thread_id = ${raceThreadId} + ORDER BY "order", step_order + `; + const raceGen = await sql<{ count: string }[]>` + SELECT count(*)::text AS count FROM app.generations + WHERE thread_id = ${raceThreadId} + `; + const raceReply = raceRows[1]; + record( + 'racing sends: one turn wins the thread, the other is refused, one full exchange lands', + raceStatuses[0] === 'completed' && + raceStatuses[1] === 'refused' && + raceHttp[0] === 200 && + raceHttp[1] === 409 && + raceRows.length === 2 && + raceRows[0]?.role === 'user' && + raceReply?.role === 'assistant' && + raceReply.status === 'complete' && + (raceReply.text ?? '').includes(`tick${SLOW_CHUNKS}`) && + raceGen[0]?.count === '0', + `outcomes=${raceStatuses.join('/')} (want completed/refused), http=${raceHttp.join('/')} (want 200/409), rows=${raceRows.length} (want 2), reply=${raceReply?.status ?? 'NONE'} full=${(raceReply?.text ?? '').includes(`tick${SLOW_CHUNKS}`)}, genGone=${raceGen[0]?.count === '0'}`, + ); } finally { await new Promise((resolve) => { aiServer.close(() => resolve()); @@ -26687,6 +26754,328 @@ async function checkChatThreadSurface( renamed[0]?.title === 'Owner named it', `title=${JSON.stringify(title)} afterRename=${renamed[0]?.title}`, ); + + // Sharing follows the conversation's lifecycle: a trashed thread's link + // goes dark, revoking still works in the trash (and answers honestly), and + // a restore brings the thread back without resurrecting a link the owner + // switched off meanwhile. + const sharedLink = (linkToken: string): Promise => + fetch(`${base}/api/app/chat/threads/shared/${linkToken}?orgId=${orgId}`, { + headers: { cookie }, + }); + const okBody = z.object({ ok: z.boolean() }); + const linked = await mkThread({ title: 'Shared then trashed' }); + const linkedShare = z + .object({ shareToken: z.string() }) + .safeParse( + await ( + await post(`/api/app/chat/threads/${linked}/share?orgId=${orgId}`) + ).json(), + ); + const linkedToken = linkedShare.success ? linkedShare.data.shareToken : ''; + const litBefore = await sharedLink(linkedToken); + await post(`/api/app/chat/threads/${linked}/trash?orgId=${orgId}`); + const darkInTrash = await sharedLink(linkedToken); + const unshareInTrash = okBody.safeParse( + await ( + await post(`/api/app/chat/threads/${linked}/unshare?orgId=${orgId}`) + ).json(), + ); + await post(`/api/app/chat/threads/${linked}/restore?orgId=${orgId}`); + const shareAfterRestore = z + .object({ isShared: z.boolean() }) + .loose() + .safeParse( + await get(`/api/app/chat/threads/${linked}/share-status?orgId=${orgId}`), + ); + const darkAfterRestore = await sharedLink(linkedToken); + const unshareUnknown = okBody.safeParse( + await ( + await post(`/api/app/chat/threads/no-such-thread/unshare?orgId=${orgId}`) + ).json(), + ); + record( + 'share links die with a trashed thread; unshare works in the trash', + litBefore.status === 200 && + darkInTrash.status === 404 && + unshareInTrash.success && + unshareInTrash.data.ok && + shareAfterRestore.success && + !shareAfterRestore.data.isShared && + darkAfterRestore.status === 404 && + unshareUnknown.success && + !unshareUnknown.data.ok, + `live=${litBefore.status} (want 200), trashed=${darkInTrash.status} (want 404), unshareInTrash=${unshareInTrash.success ? unshareInTrash.data.ok : 'ERR'} (want true), sharedAfterRestore=${shareAfterRestore.success ? shareAfterRestore.data.isShared : 'ERR'} (want false), afterRestore=${darkAfterRestore.status} (want 404), unknownThread=${unshareUnknown.success ? unshareUnknown.data.ok : 'ERR'} (want false)`, + ); + + // Refiling a project-shared thread ends the share — the new project's + // members never inherit an audience the owner consented to elsewhere — and + // the implicit unshare leaves an audit row on the project it left. threadB + // is filed in `projectId` and shared with it since the filing probe. + const summaryOf = async ( + id: string, + ): Promise<{ + projectId: string | null; + sharedWithProject: boolean | null; + pinnedAt: number | null; + } | null> => { + const parsed = z + .object({ + thread: z + .object({ + projectId: z.string().nullable(), + sharedWithProject: z.boolean().nullable(), + pinnedAt: z.number().nullable(), + }) + .loose(), + }) + .safeParse( + await get(`/api/app/chat/threads/${id}/summary?orgId=${orgId}`), + ); + return parsed.success ? parsed.data.thread : null; + }; + const projectTwo = z + .object({ projectId: z.string() }) + .safeParse( + await ( + await post(`/api/app/projects?orgId=${orgId}`, { name: 'Chat Tab Two' }) + ).json(), + ); + const projectTwoId = projectTwo.success ? projectTwo.data.projectId : ''; + const sharedBeforeMove = (await summaryOf(threadB))?.sharedWithProject; + await post(`/api/app/chat/threads/${threadB}/project?orgId=${orgId}`, { + projectId: projectTwoId, + }); + const movedSummary = await summaryOf(threadB); + const moveAudit = await sql<{ count: string }[]>` + SELECT count(*)::text AS count FROM app.audit_logs + WHERE org_id = ${orgId} AND action = 'project.thread.unshared' + AND resource_id = ${projectId} + AND new_state->>'threadId' = ${threadB} + AND new_state->>'movedToProjectId' = ${projectTwoId} + `; + const tabTwo = z + .object({ + mine: z.array( + z + .object({ id: z.string(), sharedWithProject: z.boolean().nullable() }) + .loose(), + ), + }) + .loose() + .safeParse( + await get(`/api/app/chat/project/${projectTwoId}/threads?orgId=${orgId}`), + ); + record( + 'moving a project-shared thread ends the share and audits the old project', + sharedBeforeMove === true && + movedSummary?.projectId === projectTwoId && + movedSummary.sharedWithProject === false && + moveAudit[0]?.count === '1' && + tabTwo.success && + tabTwo.data.mine.some( + (row) => row.id === threadB && row.sharedWithProject === false, + ), + `before=${sharedBeforeMove} (want true), after: project=${movedSummary?.projectId === projectTwoId} shared=${movedSummary?.sharedWithProject} (want false), audit=${moveAudit[0]?.count} (want 1), newTabShared=${tabTwo.success ? tabTwo.data.mine.find((row) => row.id === threadB)?.sharedWithProject : 'ERR'} (want false)`, + ); + + // Arena: column B is born with A's project filing and effort pick — both + // columns run with the project's instructions and knowledge — and stays + // out of the project's Chats tab while hidden; a winning B keeps the + // filing and takes over A's pin. + const arenaA = await mkThread({ + title: 'Arena in a project', + projectId: projectTwoId, + }); + await post(`/api/app/chat/threads/${arenaA}/pin?orgId=${orgId}`, { + pinned: true, + }); + await post( + `/api/app/chat/threads/${arenaA}/reasoning-effort?orgId=${orgId}`, + { + reasoningEffort: 'high', + }, + ); + const ensured = z + .object({ threadIdB: z.string() }) + .safeParse( + await ( + await post( + `/api/app/chat/threads/${arenaA}/arena/ensure?orgId=${orgId}`, + ) + ).json(), + ); + const arenaB = ensured.success ? ensured.data.threadIdB : ''; + const bBirth = await sql< + { projectId: string | null; reasoningEffort: string | null }[] + >` + SELECT project_id AS "projectId", reasoning_effort AS "reasoningEffort" + FROM app.thread_metadata WHERE thread_id = ${arenaB} + `; + const tabWhilePaired = z + .object({ mine: z.array(z.object({ id: z.string() }).loose()) }) + .loose() + .safeParse( + await get(`/api/app/chat/project/${projectTwoId}/threads?orgId=${orgId}`), + ); + const settled = z + .object({ continueThreadId: z.string() }) + .safeParse( + await ( + await post( + `/api/app/chat/threads/${arenaA}/arena/settle?orgId=${orgId}`, + { verdict: 'b_better' }, + ) + ).json(), + ); + const bAfterWin = await summaryOf(arenaB); + record( + 'arena column B carries the project; a winning B stays filed and pinned', + ensured.success && + bBirth[0]?.projectId === projectTwoId && + bBirth[0].reasoningEffort === 'high' && + tabWhilePaired.success && + !tabWhilePaired.data.mine.some((row) => row.id === arenaB) && + settled.success && + settled.data.continueThreadId === arenaB && + bAfterWin?.projectId === projectTwoId && + bAfterWin.pinnedAt !== null, + `ensured=${ensured.success}, B.project=${bBirth[0]?.projectId === projectTwoId} (want A's), B.effort=${bBirth[0]?.reasoningEffort ?? 'NULL'} (want high), hiddenFromTab=${tabWhilePaired.success ? !tabWhilePaired.data.mine.some((row) => row.id === arenaB) : 'ERR'}, settled→B=${settled.success && settled.data.continueThreadId === arenaB}, winnerFiled=${bAfterWin?.projectId === projectTwoId}, winnerPinned=${bAfterWin?.pinnedAt !== null}`, + ); + + // The chat search tool applies the contacts domain's lifecycle rule: a + // trashed contact never resurfaces in an answer. + const stamp = Date.now(); + await sql` + INSERT INTO app.contacts ( + org_id, name, email, source, lifecycle_status, created_at_ms, + updated_at_ms + ) VALUES + (${orgId}, 'Zelda Trashed', 'zelda.trashed@example.com', 'itest', + 'trashed', ${stamp}, ${stamp}), + (${orgId}, 'Zelda Current', 'zelda.current@example.com', 'itest', + NULL, ${stamp}, ${stamp}) + `; + const { chatShimHandlers } = await import('./domains/chat/shim.ts'); + const contactQuery = + chatShimHandlers(sql)['contacts/internal_queries:queryContacts']; + const contactPage = z + .object({ page: z.array(z.object({ name: z.string().optional() })) }) + .loose() + .safeParse( + contactQuery === undefined + ? null + : await contactQuery({ organizationId: orgId, searchTerm: 'Zelda' }), + ); + const contactNames = contactPage.success + ? contactPage.data.page.map((row) => row.name ?? '') + : []; + record( + 'chat search tool hides trashed contacts', + contactNames.includes('Zelda Current') && + !contactNames.includes('Zelda Trashed'), + `names=${JSON.stringify(contactNames)} (want Zelda Current only)`, + ); + + // beginTurn is ONE transaction: a crash at the generation row leaves no + // user message and no pending placeholder behind. The trigger stands in + // for the process dying between the statements. + const crashed = await mkThread({ title: 'Crash mid-open' }); + await sql.unsafe(` + CREATE OR REPLACE FUNCTION app.itest_refuse_generation() RETURNS trigger AS $$ + BEGIN + IF NEW.thread_id = '${crashed}' THEN + RAISE EXCEPTION 'itest: simulated crash before the generation row'; + END IF; + RETURN NEW; + END $$ LANGUAGE plpgsql; + CREATE TRIGGER itest_refuse_generation BEFORE INSERT ON app.generations + FOR EACH ROW EXECUTE FUNCTION app.itest_refuse_generation(); + `); + let openFailed = false; + try { + await createPgTurnStore(sql).beginTurn({ + organizationId: orgId, + threadId: crashed, + userParts: [{ type: 'text', text: 'this question must not land' }], + }); + } catch (error) { + openFailed = + error instanceof Error && /simulated crash/.test(error.message); + } finally { + await sql.unsafe(` + DROP TRIGGER IF EXISTS itest_refuse_generation ON app.generations; + DROP FUNCTION IF EXISTS app.itest_refuse_generation(); + `); + } + const crashedRows = await sql<{ count: string }[]>` + SELECT count(*)::text AS count FROM app.messages + WHERE thread_id = ${crashed} + `; + const crashedMeta = await sql<{ generationStatus: string | null }[]>` + SELECT generation_status AS "generationStatus" + FROM app.thread_metadata WHERE thread_id = ${crashed} + `; + record( + 'beginTurn is one transaction: a crash at the generation row strands nothing', + openFailed && + crashedRows[0]?.count === '0' && + crashedMeta[0]?.generationStatus !== 'generating', + `openThrew=${openFailed}, strandedRows=${crashedRows[0]?.count} (want 0), sidecar=${crashedMeta[0]?.generationStatus ?? 'NULL'} (want not generating)`, + ); + + // Two opens racing into one thread: the generation row is the claim, so + // exactly one wins; the loser rolls back (no rows) and never touches the + // winner's row. Ending the winner's turn without a settled reply fails its + // placeholder rather than leaving an eternal pending bubble. + const raced = await mkThread({ title: 'Racing opens' }); + const turnStore = createPgTurnStore(sql); + const openings = await Promise.allSettled([ + turnStore.beginTurn({ + organizationId: orgId, + threadId: raced, + userParts: [{ type: 'text', text: 'first tab' }], + }), + turnStore.beginTurn({ + organizationId: orgId, + threadId: raced, + userParts: [{ type: 'text', text: 'second tab' }], + }), + ]); + const wins = openings.filter((o) => o.status === 'fulfilled').length; + const busyLosses = openings.filter( + (o) => + o.status === 'rejected' && + o.reason instanceof Error && + o.reason.name === 'ThreadBusyError', + ).length; + const racedGens = await sql<{ count: string }[]>` + SELECT count(*)::text AS count FROM app.generations + WHERE thread_id = ${raced} + `; + const racedRows = await sql<{ count: string }[]>` + SELECT count(*)::text AS count FROM app.messages WHERE thread_id = ${raced} + `; + await turnStore.endGeneration({ organizationId: orgId, threadId: raced }); + const racedPlaceholder = await sql<{ status: string }[]>` + SELECT status FROM app.messages + WHERE thread_id = ${raced} AND role = 'assistant' + `; + const racedGensAfter = await sql<{ count: string }[]>` + SELECT count(*)::text AS count FROM app.generations + WHERE thread_id = ${raced} + `; + record( + 'racing opens: one claims the thread, the loser leaves no trace, settle fails the orphan placeholder', + wins === 1 && + busyLosses === 1 && + racedGens[0]?.count === '1' && + racedRows[0]?.count === '2' && + racedPlaceholder.length === 1 && + racedPlaceholder[0]?.status === 'failed' && + racedGensAfter[0]?.count === '0', + `wins=${wins} (want 1), busy=${busyLosses} (want 1), generations=${racedGens[0]?.count} (want 1), rows=${racedRows[0]?.count} (want 2), placeholderAfterEnd=${racedPlaceholder.map((row) => row.status).join(',')} (want failed), genAfterEnd=${racedGensAfter[0]?.count} (want 0)`, + ); } /** Re-run the guarded fill-only write against a NAMED thread — it must be a diff --git a/services/platform/lib/chat/turn.test.ts b/services/platform/lib/chat/turn.test.ts index c624dc3e0c..368b8294ec 100644 --- a/services/platform/lib/chat/turn.test.ts +++ b/services/platform/lib/chat/turn.test.ts @@ -16,6 +16,7 @@ import { LAST_TOOL_ROUND_NOTICE, MAX_TOOL_ROUNDS, runTurn, + ThreadBusyError, TOOL_BUDGET_SPENT_NOTICE, TURN_STEPS, type ModelCall, @@ -252,6 +253,31 @@ function passFilter(name: GuardrailFilter['name'] = 'pii'): GuardrailFilter { }; } +describe('runTurn — the at-most-one-turn claim', () => { + it('propagates a busy refusal from the open and never closes the other turn', async () => { + const { store, calls } = fakeStore(); + const held: TurnStore = { + ...store, + beginTurn() { + calls.ops.push('beginTurn'); + return Promise.reject(new ThreadBusyError('thread_1')); + }, + }; + const d = deps({ store: held }); + + await expect(runTurn(request(), d.deps)).rejects.toBeInstanceOf( + ThreadBusyError, + ); + + // The generation row belongs to the turn that won it: the loser runs no + // endGeneration (which would close the winner's turn), appends no + // refusal row, and never dispatches the model. + expect(calls.ops).toEqual(['beginTurn']); + expect(d.chunks).toEqual([]); + expect(d.usage).toEqual([]); + }); +}); + describe('runTurn — the happy path', () => { it('runs every step, in the contracted order', async () => { const d = deps(); diff --git a/services/platform/lib/chat/turn.ts b/services/platform/lib/chat/turn.ts index 5aa4e40062..3bbe5fbc05 100644 --- a/services/platform/lib/chat/turn.ts +++ b/services/platform/lib/chat/turn.ts @@ -242,6 +242,12 @@ export interface TurnStore { * message-info panel reports is their sum. Atomic, so a failure can never * strand a user message whose reply will not arrive, or a placeholder with * no generation row. + * + * The generation row IS the at-most-one-turn gate: opening it is a claim, + * and a thread whose row already exists rejects with {@link + * ThreadBusyError} — the whole open rolls back, so the loser leaves no + * message behind and never touches the winner's row. A read-before-write + * check in the caller is only a fast path; this is the guard. */ beginTurn(setup: { organizationId: string; @@ -265,6 +271,28 @@ export interface TurnStore { }): Promise; } +/** The one sentence every lane answers a concurrent send with. */ +export const THREAD_BUSY_REASON = + 'This conversation is already generating a response.'; + +/** + * `beginTurn` found another turn's generation row on the thread and opened + * nothing: its transaction rolled back, so the losing send leaves no user + * message, no placeholder, and never rebinds or deletes the winner's row. + * Raised BEFORE the pipeline's settle block, so the loser never runs the + * `endGeneration` that would close the winner's turn. Hosts answer it as a + * refusal, never as an internal error. + */ +export class ThreadBusyError extends Error { + readonly code = 'THREAD_BUSY' as const; + readonly threadId: string; + constructor(threadId: string) { + super(THREAD_BUSY_REASON); + this.name = 'ThreadBusyError'; + this.threadId = threadId; + } +} + export interface UsageLedgerEntry { readonly organizationId: string; readonly userId: string;