diff --git a/apps/server/src/plugins/chat-hooks.ts b/apps/server/src/plugins/chat-hooks.ts new file mode 100644 index 0000000..3fe8b9d --- /dev/null +++ b/apps/server/src/plugins/chat-hooks.ts @@ -0,0 +1,24 @@ +import { nsid } from '@scn-chat/lexicons' +import type { Logger } from '../logger.ts' +import { spaceUri } from '../storage/records.ts' +import type { SyncEventBus } from '../sync/events.ts' +import type { HookRunner } from './hooks.ts' + +/** + * Tell plugins about each conversation created or deleted, whether this app made the change or + * sync noticed another client make it. The events come once per change, so the hooks do too. + */ +export function fireChatHooks(events: SyncEventBus, hooks: () => HookRunner, logger: Logger) { + const conversation = (did: string, skey: string) => ({ + user: did, + conversation: spaceUri(did, nsid.conversation, skey), + }) + events.on( + 'conversation:created', + ({ did, skey }) => void hooks().action('conversation:created', conversation(did, skey), logger), + ) + events.on( + 'conversation:deleted', + ({ did, skey }) => void hooks().action('conversation:deleted', conversation(did, skey), logger), + ) +} diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 059a125..789de8c 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -16,6 +16,7 @@ import type { Db } from './db/index.ts' import { migrateToLatest } from './db/migrate.ts' import type { Logger } from './logger.ts' import { PluginAdmin } from './plugins/admin.ts' +import { fireChatHooks } from './plugins/chat-hooks.ts' import type { PluginServices } from './plugins/host.ts' import type { InstalledPlugins } from './plugins/installed.ts' import { readInstances } from './plugins/instances.ts' @@ -183,6 +184,7 @@ export async function createServer(deps: ServerDeps) { logger, }) runner.attach(events) + fireChatHooks(events, () => runtime().host.hooks, logger) const sharing = new SharingService({ getPdsClient, identity, @@ -241,7 +243,6 @@ export async function createServer(deps: ServerDeps) { services, events, engine, - hooks: () => runtime().host.hooks, sync: () => settings.get('sync'), logger, }, diff --git a/apps/server/src/storage/chat-service.ts b/apps/server/src/storage/chat-service.ts index 8885bcb..f0e285b 100644 --- a/apps/server/src/storage/chat-service.ts +++ b/apps/server/src/storage/chat-service.ts @@ -40,6 +40,14 @@ export type ConversationChanges = { full: boolean } +/** What a ChatService tells its owner about the changes it makes. */ +export type ChatListener = { + /** A record was written to a space. */ + wrote?: (space: string, cid: string) => void + created?: (skey: string) => void + deleted?: (skey: string) => void +} + export type IndexChanges = { conversations: ConversationSummary[] deleted: string[] @@ -57,19 +65,14 @@ export class ChatService { readonly settingsUri: string private readonly blobs: BlobStore private readonly logger: Logger - private readonly onWrite: (space: string, cid: string) => void + private readonly listener: ChatListener - constructor( - store: RecordStore, - blobs: BlobStore, - logger: Logger, - onWrite: (space: string, cid: string) => void = () => {}, - ) { + constructor(store: RecordStore, blobs: BlobStore, logger: Logger, listener: ChatListener = {}) { this.did = store.did this.store = store this.blobs = blobs this.logger = logger - this.onWrite = onWrite + this.listener = listener this.settingsUri = spaceUri(store.did, nsid.settings, SETTINGS_SKEY) } @@ -116,7 +119,7 @@ export class ChatService { const { cid } = create ? await this.store.createRecord(space, collection, rkey, value) : await this.store.putRecord(space, collection, rkey, value) - this.onWrite(space, cid) + this.listener.wrote?.(space, cid) return cid } @@ -128,6 +131,7 @@ export class ChatService { if (input.systemPrompt) info.systemPrompt = input.systemPrompt await this.put(uri, nsid.info, 'self', info) await this.inSettingsSpace(() => this.writeRef(skey, { tags: input.tags ?? [] })) + this.listener.created?.(skey) return { skey, uri } } @@ -225,6 +229,7 @@ export class ChatService { async deleteConversation(skey: string): Promise { await this.store.deleteSpace(this.conversationUri(skey)) await this.store.deleteRecord(this.settingsUri, nsid.conversationRef, skey) + this.listener.deleted?.(skey) } async getPreferences(): Promise { diff --git a/apps/server/src/storage/routes.ts b/apps/server/src/storage/routes.ts index fb6284a..d110d3d 100644 --- a/apps/server/src/storage/routes.ts +++ b/apps/server/src/storage/routes.ts @@ -7,7 +7,6 @@ import { jsonBody } from '../body.ts' import type { Db } from '../db/index.ts' import type { AppEnv } from '../env.ts' import type { Logger } from '../logger.ts' -import type { HookRunner } from '../plugins/hooks.ts' import { jsonObject } from '../schemas.ts' import type { Settings } from '../settings/schemas.ts' import type { SyncEngine } from '../sync/engine.ts' @@ -19,8 +18,6 @@ export type StorageRoutesDeps = { services: ChatServices events: SyncEventBus engine?: SyncEngine - /** The current plugin runtime's hooks. */ - hooks?: () => HookRunner /** The current sync settings. */ sync: () => Settings['sync'] logger: Logger @@ -61,9 +58,6 @@ export function storageRoutes(deps: StorageRoutesDeps) { systemPrompt: body.systemPrompt, tags: body.tags, }) - await deps - .hooks?.() - .action('conversation:created', { user: chats.did, conversation: created.uri }, deps.logger) return c.json(created, 201) }) .get('/conversations/keys', async (c) => c.json({ keys: await chatsFor(c).listKeys() })) @@ -94,16 +88,7 @@ export function storageRoutes(deps: StorageRoutesDeps) { return c.json({ ok: true }) }) .delete('/conversations/:skey', async (c) => { - const chats = chatsFor(c) - const skey = c.req.param('skey') - await chats.deleteConversation(skey) - await deps - .hooks?.() - .action( - 'conversation:deleted', - { user: chats.did, conversation: chats.conversationUri(skey) }, - deps.logger, - ) + await chatsFor(c).deleteConversation(c.req.param('skey')) return c.json({ ok: true }) }) .post('/conversations/:skey/sync', async (c) => { diff --git a/apps/server/src/storage/services.ts b/apps/server/src/storage/services.ts index 174e647..0aa3190 100644 --- a/apps/server/src/storage/services.ts +++ b/apps/server/src/storage/services.ts @@ -40,20 +40,19 @@ export function createChatServices(deps: { storeFor, forAccount(account) { const store = storeFor(account) - const chats: ChatService = new ChatService( - store, - blobsFor(account), - deps.logger, - (space, cid) => { + const { did } = account + const chats: ChatService = new ChatService(store, blobsFor(account), deps.logger, { + wrote(space, cid) { deps.recentWrites.add(cid) - if (space === chats.settingsUri) deps.events.emit('index:changed', { did: account.did }) - else - deps.events.emit('conversation:changed', { - did: account.did, - skey: chats.skeyOf(space), - }) + if (space === chats.settingsUri) deps.events.emit('index:changed', { did }) + else deps.events.emit('conversation:changed', { did, skey: chats.skeyOf(space) }) }, - ) + created: (skey) => deps.events.emit('conversation:created', { did, skey }), + deleted(skey) { + deps.recentWrites.addDeletion(chats.conversationUri(skey)) + deps.events.emit('conversation:deleted', { did, skey }) + }, + }) return chats }, } diff --git a/apps/server/src/sync/engine.ts b/apps/server/src/sync/engine.ts index e87d94c..e3e5b24 100644 --- a/apps/server/src/sync/engine.ts +++ b/apps/server/src/sync/engine.ts @@ -8,7 +8,7 @@ import type { Logger } from '../logger.ts' import type { Loose } from '../loose.ts' import type { ChatService } from '../storage/chat-service.ts' import { type OpsPage, type RecordStore, SpaceNotFound } from '../storage/record-store.ts' -import type { JsonRecord } from '../storage/records.ts' +import { type JsonRecord, spaceUri } from '../storage/records.ts' import type { ChatServices } from '../storage/services.ts' import { Coalescer } from './coalescer.ts' import type { CredentialCache } from './credentials.ts' @@ -110,6 +110,17 @@ export class SyncEngine { return this.coalescer.request(`index ${did}`, () => this.runIndexSync(did, false)) } + /** + * Forget a conversation another client deleted, as an index sync, a conversation sync, or the + * PDS's notification found, and say so once. A deletion this server made was already announced. + */ + async conversationDeleted(did: string, skey: string): Promise { + const space = spaceUri(did, nsid.conversation, skey) + await this.dropState(space) + if (!this.deps.recentWrites.hasDeletion(space)) + this.deps.events.emit('conversation:deleted', { did, skey }) + } + /** Sync one conversation now. */ async syncConversation(did: string, skey: string): Promise { const ctx = await this.context(did) @@ -127,9 +138,11 @@ export class SyncEngine { for (const op of page.ops) { if (op.collection !== nsid.conversationRef) continue if (!op.cid) { - await this.dropState(ctx.chats.conversationUri(op.rkey)) - this.deps.events.emit('conversation:deleted', { did, skey: op.rkey }) + await this.conversationDeleted(did, op.rkey) } else if (!this.deps.recentWrites.has(op.cid)) { + // A new index entry from another client is a conversation created there. + if (op.prev === null) + this.deps.events.emit('conversation:created', { did, skey: op.rkey }) await this.runConversationSync(ctx, op.rkey, false) } } @@ -168,8 +181,7 @@ export class SyncEngine { page = await ctx.store.listOps(space, state?.last_rev) } catch (err) { if (!(err instanceof SpaceNotFound)) throw err - await this.dropState(space) - this.deps.events.emit('conversation:deleted', { did, skey }) + await this.conversationDeleted(did, skey) return } let external = false diff --git a/apps/server/src/sync/events.ts b/apps/server/src/sync/events.ts index fc7d56e..11d6856 100644 --- a/apps/server/src/sync/events.ts +++ b/apps/server/src/sync/events.ts @@ -15,6 +15,9 @@ export type SyncEvents = { ] 'message:invalid': [{ did: string; skey: string; rkey: string; raw: JsonRecord; error: string }] 'conversation:changed': [{ did: string; skey: string }] + /** A conversation was created, by this app or, as sync noticed, another client. Once each. */ + 'conversation:created': [{ did: string; skey: string }] + /** A conversation was deleted, by this app or, as sync noticed, another client. Once each. */ 'conversation:deleted': [{ did: string; skey: string }] 'index:changed': [{ did: string }] } diff --git a/apps/server/src/sync/recent-writes.ts b/apps/server/src/sync/recent-writes.ts index e9a8187..cddc0e8 100644 --- a/apps/server/src/sync/recent-writes.ts +++ b/apps/server/src/sync/recent-writes.ts @@ -1,4 +1,4 @@ -/** CIDs of records the server wrote recently, so sync can skip its own changes. */ +/** Records the server wrote and spaces it deleted recently, so sync can skip its own changes. */ export class RecentWrites { private readonly entries = new Map() private readonly ttlMs: number @@ -20,6 +20,15 @@ export class RecentWrites { return false } + /** Remember a space this server deleted, so sync can tell the deletion is its own. */ + addDeletion(space: string, now = Date.now()): void { + this.add(`deleted ${space}`, now) + } + + hasDeletion(space: string, now = Date.now()): boolean { + return this.has(`deleted ${space}`, now) + } + private prune(now: number): void { for (const [cid, expires] of this.entries) if (expires <= now) this.entries.delete(cid) } diff --git a/apps/server/src/sync/routes.ts b/apps/server/src/sync/routes.ts index fd70372..7075de7 100644 --- a/apps/server/src/sync/routes.ts +++ b/apps/server/src/sync/routes.ts @@ -69,8 +69,8 @@ export function syncRoutes(deps: SyncRoutesDeps) { const { did, type, skey } = authorityOf(space) const refusal = await authorize(c.req.header('authorization'), 'notifySpaceDeleted', did) if (refusal) return c.json(refusal, 401) - await deps.db.deleteFrom('sync_state').where('space_uri', '=', space).execute() - if (type === nsid.conversation) deps.events.emit('conversation:deleted', { did, skey }) + if (type === nsid.conversation) await deps.engine.conversationDeleted(did, skey) + else await deps.db.deleteFrom('sync_state').where('space_uri', '=', space).execute() return c.body(null, 200) }) .post(`/xrpc/${nsid.requestSync}`, async (c) => { diff --git a/apps/server/tests/storage/chat-service.test.ts b/apps/server/tests/storage/chat-service.test.ts index d39ccf0..0778562 100644 --- a/apps/server/tests/storage/chat-service.test.ts +++ b/apps/server/tests/storage/chat-service.test.ts @@ -33,9 +33,9 @@ describe.each(dialects)('ChatService on $name', ({ create }) => { await migrateToLatest(db) const writes: string[] = [] const blobs = new LocalBlobStore(did, mkdtempSync(join(tmpdir(), 'scn-'))) - const chats = new ChatService(new LocalRecordStore(did, db), blobs, logger, (_space, cid) => - writes.push(cid), - ) + const chats = new ChatService(new LocalRecordStore(did, db), blobs, logger, { + wrote: (_space, cid) => writes.push(cid), + }) return { db, chats, writes } } diff --git a/apps/server/tests/sync/chat-hooks.test.ts b/apps/server/tests/sync/chat-hooks.test.ts new file mode 100644 index 0000000..744e1f3 --- /dev/null +++ b/apps/server/tests/sync/chat-hooks.test.ts @@ -0,0 +1,102 @@ +import { nsid } from '@scn-chat/lexicons' +import { describe, expect, it, vi } from 'vitest' +import { fireChatHooks } from '../../src/plugins/chat-hooks.ts' +import type { HookRunner } from '../../src/plugins/hooks.ts' +import { ALICE, collect, spacesHarness } from '../helpers/spaces.ts' + +type Harness = Awaited> + +/** Create a conversation as another client would, straight on the PDS, with its index entry. */ +async function createAsClient(h: Harness, skey: string) { + const uri = await h.external.createSpace(nsid.conversation, skey) + await h.external.createRecord(uri, nsid.info, 'self', { + $type: nsid.info, + createdAt: new Date().toISOString(), + } as never) + await h.external.putRecord(h.chats.settingsUri, nsid.conversationRef, skey, { + $type: nsid.conversationRef, + conversation: uri, + updatedAt: new Date().toISOString(), + } as never) +} + +/** Every created and deleted event, in order. */ +function changes(h: Harness) { + const seen: string[] = [] + h.events.on('conversation:created', ({ skey }) => seen.push(`created ${skey}`)) + h.events.on('conversation:deleted', ({ skey }) => seen.push(`deleted ${skey}`)) + return seen +} + +describe('conversation created and deleted', () => { + it('come once each for a conversation the app creates and deletes, even once sync sees them', async () => { + const h = await spacesHarness() + await h.engine.syncIndex(ALICE) + const seen = changes(h) + const { skey } = await h.chats.createConversation() + await h.engine.syncIndex(ALICE) + await h.chats.deleteConversation(skey) + await h.engine.syncIndex(ALICE) + await h.engine.conversationDeleted(ALICE, skey) + expect(seen).toEqual([`created ${skey}`, `deleted ${skey}`]) + }) + + it('come once each for a conversation another client creates and deletes', async () => { + const h = await spacesHarness() + // An index with a chat in it, so the server's first sync leaves a cursor to sync on from. + await h.chats.createConversation() + await h.engine.syncIndex(ALICE) + const seen = changes(h) + await createAsClient(h, '3ccccccccccc1') + await h.engine.syncIndex(ALICE) + await h.engine.syncIndex(ALICE) + await h.external.deleteRecord(h.chats.settingsUri, nsid.conversationRef, '3ccccccccccc1') + await h.engine.syncIndex(ALICE) + expect(seen).toEqual(['created 3ccccccccccc1', 'deleted 3ccccccccccc1']) + }) + + it('do not come for conversations already there when the server first syncs', async () => { + const h = await spacesHarness() + await createAsClient(h, '3ccccccccccc1') + const created = collect(h.events, 'conversation:created') + await h.engine.syncIndex(ALICE) + expect(created).toEqual([]) + }) + + it('do not come for another client updating a conversation', async () => { + const h = await spacesHarness() + await createAsClient(h, '3ccccccccccc1') + await h.engine.syncIndex(ALICE) + const seen = changes(h) + await h.external.putRecord(h.chats.settingsUri, nsid.conversationRef, '3ccccccccccc1', { + $type: nsid.conversationRef, + conversation: h.chats.conversationUri('3ccccccccccc1'), + updatedAt: new Date().toISOString(), + title: 'Renamed', + } as never) + await h.engine.syncIndex(ALICE) + expect(seen).toEqual([]) + }) + + it('come for a fallback user, whose chats only the app writes', async () => { + const h = await spacesHarness({ storageMode: 'local' }) + const seen = changes(h) + const { skey } = await h.chats.createConversation() + await h.chats.deleteConversation(skey) + expect(seen).toEqual([`created ${skey}`, `deleted ${skey}`]) + }) +}) + +describe('fireChatHooks', () => { + it("runs the plugins' hooks with the owner and the conversation's URI", async () => { + const h = await spacesHarness() + const action = vi.fn(async () => []) + fireChatHooks(h.events, () => ({ action }) as unknown as HookRunner, h.logger) + const { skey, uri } = await h.chats.createConversation() + await h.chats.deleteConversation(skey) + expect(action.mock.calls).toEqual([ + ['conversation:created', { user: ALICE, conversation: uri }, h.logger], + ['conversation:deleted', { user: ALICE, conversation: uri }, h.logger], + ]) + }) +}) diff --git a/docs/plugins.md b/docs/plugins.md index d02b571..c98cca7 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -193,14 +193,15 @@ See [`plugins/pdf-text`](../plugins/pdf-text/src/index.ts) as an example. | `messages:beforeModel` | filter | Instructions and messages about to be sent to the model, and the turn context. | | `message:afterModel` | filter | Reply's final content, before it's written to the PDS, and the turn context. | | `turn:after` | action | The completed turn, including the written reply. | -| `conversation:created` | action | The owner's DID and the conversation URI. | -| `conversation:deleted` | action | The owner's DID and the conversation URI. | +| `conversation:created` | action | The owner's DID and the conversation URI, once per new conversation. | +| `conversation:deleted` | action | The owner's DID and the conversation URI, once per deleted conversation. | | `signIn:before` | action | The DID, handle, and PDS of the user signing in. | | `cron` | action | Cron run start time. | - **Filters:** Each handlers return value is sent to the next filter. - **Actions:** Return values are ignored. If an action raises an exception it's logged and the turn and later actions continue. - **Order:** Pass `{ order: 'pre' }` or `{ order: 'post' }` to run before or after the `'normal'` handlers. +- **Conversations from any client:** `conversation:created` and `conversation:deleted` run for a change made in this app and for one another client makes on the user's PDS, once sync notices it. That is soon after the change when the PDS can notify the app, and at the next sync otherwise, as in local development. Conversations already there when a user first signs in don't count as created. - **Turn context:** `turn.tools` lists the tools offered to the model this turn, so a `messages:beforeModel` filter can add guidance for its tools only when they are in use. ## Per-user settings diff --git a/specs/plugins.md b/specs/plugins.md index e6f1b1d..42ee9ff 100644 --- a/specs/plugins.md +++ b/specs/plugins.md @@ -96,13 +96,14 @@ Hooks let many plugins act on the same event. Every payload uses our own normali | `messages:beforeModel` | filter | The system prompt and message list about to be sent, with the turn context | | `message:afterModel` | filter | The assistant's final content before it is written, with the turn context | | `turn:after` | action | The finished turn: conversation, user, request, and the written reply | -| `conversation:created` | action | The new conversation and its owner | -| `conversation:deleted` | action | The deleted conversation and its owner | +| `conversation:created` | action | The new conversation and its owner, once, whichever client created it | +| `conversation:deleted` | action | The deleted conversation and its owner, once, whichever client deleted it | | `signIn:before` | action | The DID, handle, and PDS of someone signing in, before the access check. See the scn-member-registry spec. | | `cron` | action | When the cron run started. See the cron spec. | - **Filters** run in order, and each handler returns the value the next one receives. A filter that throws fails the turn, which ends with status `error` and an error naming the plugin. - **Actions** run in order, and their return values are ignored. An action that throws is logged with the plugin ID, and the turn is unaffected. +- **Conversation hooks** come from the in-process event bus, through `fireChatHooks`, not from the routes. `ChatService` announces the conversations this app creates and deletes. Sync announces a conversation another client created when an index sync finds a new `conversationRef`, one whose op has no previous CID, and one another client deleted when an index sync finds its entry deleted, a conversation's space is gone, or the PDS notifies a deletion. The first index sync after sign-in, and any while the index is still empty, treats what it finds as already there, so it announces nothing. The server remembers the spaces it deleted for ten minutes, as it remembers its own writes, so sync noticing its own deletion doesn't announce it again. A fallback user's chats are written only by this app, so its own announcements cover them. - **Order.** Handlers are sorted by `options.order` (`'pre'`, `'normal'`, or `'post'`, default `'normal'`), then by the plugin's position in the admin area. There are no numeric priorities. The server logs each hook's resolved handler order whenever the plugins load. The hook runner is a small typed module in the server, about 80 lines, that handles ordering, filter chaining, and action error isolation. The `hookable` library has no ordering and cannot pass a value through a chain of handlers, which filters need. `tapable` has both, but needs a class per hook. @@ -152,3 +153,4 @@ Per-user settings are stored in a `plugin_user_settings` table: DID, plugin ID, - [ ] Per-user secrets no key can decrypt read as unset with `secretsUnreadable`, and are kept until the user enters a secret. - [ ] Deleting a plugin's settings clears the user's stored values, so the schema defaults apply. - [ ] `ctx.conversations.updateInfo` writes through the storage layer under the user's DID. +- [x] `conversation:created` and `conversation:deleted` run once per change, for a conversation this app creates or deletes and for one another client does, and not for conversations already there at the first sync.