diff --git a/open-sse/executors/notion-web.ts b/open-sse/executors/notion-web.ts index 57ca5b4e96..707c403621 100644 --- a/open-sse/executors/notion-web.ts +++ b/open-sse/executors/notion-web.ts @@ -77,6 +77,7 @@ export { parseNotionInferenceStream, resolveNotionThreadBinding, notionThreadMarkCreateAttempted, + notionThreadMarkConfirmed, sanitizeNotionAssistantText, }; @@ -583,11 +584,12 @@ export class NotionWebExecutor extends BaseExecutor { const clientFacing = clientFacingModelId(model); const modelId = clientFacing || notionCodename || "notion-ai"; - // Thread continuity (sticky): + // Thread continuity (sticky) — see resolveNotionThreadBinding: // - Prefer X-Notion-Thread-Id / body pin from the client - // - Else sticky root key from first user message (UREW-normalized, durable on disk) - // - Bind threadId *before* the upstream call so error retries never mint a new chat - // - createThread:true only for brand-new roots; never again for that root + // - Else exact conversation-prefix hash (multi-turn OpenAI history) + // - Else sticky root (first user text) for UREW + failed-first-request retries + // - First-turn + confirmed sticky (new Claude Code session with same “hi”) → mint fresh + // - Bind threadId *before* the upstream call so error retries never mint a second chat const inboundHeaders = (input.clientHeaders as Record | null | undefined) ?? ((input as { headers?: Record }).headers as diff --git a/open-sse/services/notionThreadSessions.ts b/open-sse/services/notionThreadSessions.ts index 2aaeac14eb..f1e123a17f 100644 --- a/open-sse/services/notionThreadSessions.ts +++ b/open-sse/services/notionThreadSessions.ts @@ -269,9 +269,18 @@ export function notionThreadRootKey(spaceKey: string, messages: NotionMessage[]) /** * Resolve which Notion thread to use and whether to mint a new one. - * - Sticky root binding is written *before* the upstream call so errors/retries - * never open a second Notion chat for the same conversation. - * - Any prior assistant history forces createThread:false when a sticky id exists. + * + * Continuity rules (order matters): + * 1. Client-supplied thread id (body/header pin) → always follow-up. + * 2. Exact conversation-prefix hash → multi-turn OpenAI history (most specific). + * 3. Sticky root (first-user-message key): + * - Multi-turn history present → reuse (UREW-resilient when prefix hash misses). + * - First turn + createAttempted && !confirmed → error-retry stickiness + * (never mint a second Notion chat for the same failed first request). + * - First turn + confirmed → NEW session with the same opener text (e.g. + * Claude Code “new session” + “hi” again). Must mint a fresh threadId — + * reusing the confirmed sticky forks the previous Notion chat. + * 4. Otherwise mint createThread:true and bind optimistically. */ export function resolveNotionThreadBinding( spaceKey: string, @@ -288,27 +297,10 @@ export function resolveNotionThreadBinding( return { threadId: id, createThread: false, rootKey }; } - // Prefer sticky root (survives UREW rewrites + error retries) - if (rootKey) { - const sticky = readThreadSessionEntry(rootKey); - if (sticky?.threadId) { - // Touch TTL - putThreadSession(rootKey, sticky.threadId, { - confirmed: sticky.confirmed, - createAttempted: sticky.createAttempted, - }); - // If we already attempted create for this root, never create again - // (even when the first reply failed — Notion may already have the thread). - const createThread = !sticky.createAttempted && !sticky.confirmed && !hasHistory; - return { - threadId: sticky.threadId, - createThread, - rootKey, - }; - } - } - - // Exact prefix match (full history before last user) + // Exact prefix match first (full history before last user) — most specific + // multi-turn continuity. Prefer this over sticky root so two independent + // sessions that share the same first-user opener do not steal each other's + // sticky binding when both are multi-turn. const prefix = conversationPrefixBeforeLastUser(messages); if (prefix.length > 0) { const exactId = readThreadSession(hashNotionConversation(spaceKey, prefix)); @@ -318,6 +310,61 @@ export function resolveNotionThreadBinding( } } + // Sticky root (first user turn hash) — UREW + error-retry continuity + if (rootKey) { + const sticky = readThreadSessionEntry(rootKey); + if (sticky?.threadId) { + // Multi-turn OpenAI history → continue the sticky Notion chat + // (covers UREW rewrites where prefix hash may not match turn-1 store). + if (hasHistory) { + putThreadSession(rootKey, sticky.threadId, { + confirmed: sticky.confirmed, + createAttempted: sticky.createAttempted, + }); + return { + threadId: sticky.threadId, + createThread: false, + rootKey, + }; + } + + // First-turn error retry: we already issued createThread:true for this + // root but never got a successful reply. Keep the same threadId so Notion + // is not spam-created; do not create again (Notion may already have it). + if (sticky.createAttempted && !sticky.confirmed) { + putThreadSession(rootKey, sticky.threadId, { + confirmed: false, + createAttempted: true, + }); + return { + threadId: sticky.threadId, + createThread: false, + rootKey, + }; + } + + // First-turn + confirmed sticky: a *new* client session that happens to + // start with the same first user text (Claude Code “New session” + “hi”). + // Fall through and mint — never fork the previous Notion thread. + // + // Optimistic pre-bind (createAttempted false, confirmed false) also falls + // through only when no sticky exists; if sticky exists without either flag + // it is mid-flight first bind — reuse with createThread:true once. + if (!sticky.createAttempted && !sticky.confirmed) { + putThreadSession(rootKey, sticky.threadId, { + confirmed: false, + createAttempted: false, + }); + return { + threadId: sticky.threadId, + createThread: true, + rootKey, + }; + } + // sticky.confirmed on first-turn → mint below (rebind root to new id) + } + } + // Mint a new thread id and bind it immediately (optimistic) so concurrent / // failed retries reuse the same id instead of spam-creating Notion chats. const threadId = randomUUID(); diff --git a/tests/unit/executor-notion-web-thread-sessions.test.ts b/tests/unit/executor-notion-web-thread-sessions.test.ts index 7f8bde63f5..14764c3ad4 100644 --- a/tests/unit/executor-notion-web-thread-sessions.test.ts +++ b/tests/unit/executor-notion-web-thread-sessions.test.ts @@ -34,6 +34,29 @@ function installNotionTlsMock( return () => __setTlsFetchOverrideForTesting(null); } +function okNdjson(text: string): string { + return [ + JSON.stringify({ type: "patch-start", data: { s: [] } }), + JSON.stringify({ + type: "record-map", + recordMap: { + thread_message: { + m1: { + value: { + value: { + step: { + type: "agent-inference", + value: [{ type: "text", content: text }], + }, + }, + }, + }, + }, + }, + }), + ].join("\n"); +} + describe("Notion thread session continuity", () => { const { __resetNotionThreadSessionsForTests, @@ -144,7 +167,7 @@ describe("Notion thread session continuity", () => { const executor = new NotionWebExecutor(); const captured: Array<{ createThread?: boolean; threadId?: string }> = []; let n = 0; - const restore = installNotionTlsMock(async (_url, opts) => { + const restoreTls = installNotionTlsMock(async (_url, opts) => { const body = JSON.parse(String(opts.body)) as { createThread?: boolean; threadId?: string; @@ -163,27 +186,7 @@ describe("Notion thread session continuity", () => { }), }; } - const ndjson = [ - JSON.stringify({ type: "patch-start", data: { s: [] } }), - JSON.stringify({ - type: "record-map", - recordMap: { - thread_message: { - m1: { - value: { - value: { - step: { - type: "agent-inference", - value: [{ type: "text", content: "recovered" }], - }, - }, - }, - }, - }, - }, - }), - ].join("\n"); - return { status: 200, text: ndjson }; + return { status: 200, text: okNdjson("recovered") }; }); try { const result = await executor.execute({ @@ -201,7 +204,90 @@ describe("Notion thread session continuity", () => { const json = (await result.response.json()) as { choices?: { message?: { content?: string } }[] }; assert.match(String(json.choices?.[0]?.message?.content || ""), /recovered/); } finally { - restore(); + restoreTls(); + __resetNotionThreadSessionsForTests(); + } + }); + + it("new first-turn after confirmed chat with same opener mints a fresh thread", () => { + __resetNotionThreadSessionsForTests(); + const { + resolveNotionThreadBinding, + notionThreadMarkCreateAttempted, + notionThreadMarkConfirmed, + } = mod as typeof mod & { + resolveNotionThreadBinding: ( + spaceKey: string, + messages: { role: string; content: string }[], + clientThreadId?: string + ) => { threadId: string; createThread: boolean; rootKey: string | null }; + notionThreadMarkCreateAttempted: (rootKey: string | null, threadId: string) => void; + notionThreadMarkConfirmed: (rootKey: string | null, threadId: string) => void; + }; + + const spaceId = "space-new-session"; + const hi = [{ role: "user", content: "hi" }]; + + const b1 = resolveNotionThreadBinding(spaceId, hi); + assert.equal(b1.createThread, true); + notionThreadMarkCreateAttempted(b1.rootKey, b1.threadId); + notionThreadMarkConfirmed(b1.rootKey, b1.threadId); + + // Claude Code "New session" + same first message must NOT fork the prior Notion chat + const b2 = resolveNotionThreadBinding(spaceId, hi); + assert.equal(b2.createThread, true); + assert.notEqual(b2.threadId, b1.threadId); + + // Multi-turn of the *new* session still sticks to b2 via prefix / sticky history + mod.notionThreadSessionStore( + spaceId, + [{ role: "user", content: "hi" }], + "hello from session 2", + b2.threadId + ); + const multi = [ + { role: "user", content: "hi" }, + { role: "assistant", content: "hello from session 2" }, + { role: "user", content: "next" }, + ]; + const b3 = resolveNotionThreadBinding(spaceId, multi); + assert.equal(b3.createThread, false); + assert.equal(b3.threadId, b2.threadId); + }); + + it("execute: two sequential first-turns with same text get distinct Notion threads", async () => { + __resetNotionThreadSessionsForTests(); + const executor = new mod.NotionWebExecutor(); + const captured: Array<{ createThread?: boolean; threadId?: string }> = []; + const restoreTls = installNotionTlsMock(async (_url, opts) => { + captured.push(JSON.parse(String(opts.body))); + return { status: 200, text: okNdjson("pong") }; + }); + try { + const creds = { apiKey: COOKIE_WITH_SPACE }; + const r1 = await executor.execute({ + model: "fable-5", + body: { messages: [{ role: "user", content: "hi" }] }, + stream: false, + credentials: creds, + signal: null, + } as never); + assert.equal(r1.response.status, 200); + assert.equal(captured[0]!.createThread, true); + + // Brand-new Claude Code session, same opener text only + const r2 = await executor.execute({ + model: "fable-5", + body: { messages: [{ role: "user", content: "hi" }] }, + stream: false, + credentials: creds, + signal: null, + } as never); + assert.equal(r2.response.status, 200); + assert.equal(captured[1]!.createThread, true); + assert.notEqual(captured[0]!.threadId, captured[1]!.threadId); + } finally { + restoreTls(); __resetNotionThreadSessionsForTests(); } }); @@ -229,29 +315,9 @@ describe("Notion thread session continuity", () => { __resetNotionThreadSessionsForTests(); const executor = new mod.NotionWebExecutor(); const captured: Array<{ createThread?: boolean; threadId?: string }> = []; - const restore = installNotionTlsMock(async (_url, opts) => { + const restoreTls = installNotionTlsMock(async (_url, opts) => { captured.push(JSON.parse(String(opts.body))); - const ndjson = [ - JSON.stringify({ type: "patch-start", data: { s: [] } }), - JSON.stringify({ - type: "record-map", - recordMap: { - thread_message: { - m1: { - value: { - value: { - step: { - type: "agent-inference", - value: [{ type: "text", content: "ok" }], - }, - }, - }, - }, - }, - }, - }), - ].join("\n"); - return { status: 200, text: ndjson }; + return { status: 200, text: okNdjson("ok") }; }); try { const r1 = await executor.execute({ @@ -262,8 +328,8 @@ describe("Notion thread session continuity", () => { signal: null, } as never); assert.equal(r1.response.status, 200); - assert.equal(captured[0].createThread, true); - const t1 = captured[0].threadId; + assert.equal(captured[0]!.createThread, true); + const t1 = captured[0]!.threadId; assert.ok(t1 && t1.length > 10); const json1 = (await r1.response.json()) as { notion_thread_id?: string; id?: string }; @@ -283,10 +349,10 @@ describe("Notion thread session continuity", () => { signal: null, } as never); assert.equal(r2.response.status, 200); - assert.equal(captured[1].createThread, false); - assert.equal(captured[1].threadId, t1); + assert.equal(captured[1]!.createThread, false); + assert.equal(captured[1]!.threadId, t1); } finally { - restore(); + restoreTls(); __resetNotionThreadSessionsForTests(); } }); @@ -297,34 +363,14 @@ describe("Notion thread session continuity", () => { const pinned = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; let capturedCreateThread: boolean | undefined; let capturedThreadId: string | undefined; - const restore = installNotionTlsMock(async (_url, opts) => { + const restoreTls = installNotionTlsMock(async (_url, opts) => { const body = JSON.parse(String(opts.body)) as { createThread?: boolean; threadId?: string; }; capturedCreateThread = body.createThread; capturedThreadId = body.threadId; - const ndjson = [ - JSON.stringify({ type: "patch-start", data: { s: [] } }), - JSON.stringify({ - type: "record-map", - recordMap: { - thread_message: { - m1: { - value: { - value: { - step: { - type: "agent-inference", - value: [{ type: "text", content: "ok" }], - }, - }, - }, - }, - }, - }, - }), - ].join("\n"); - return { status: 200, text: ndjson }; + return { status: 200, text: okNdjson("ok") }; }); try { // Real ExecuteInput shape: clientHeaders only (headers is undefined). @@ -342,7 +388,7 @@ describe("Notion thread session continuity", () => { // Client-supplied thread id must force follow-up mode (createThread=false). assert.equal(capturedCreateThread, false); } finally { - restore(); + restoreTls(); __resetNotionThreadSessionsForTests(); } });