From c9ab955a2ab340ab215fcfc62af266f23259b8d4 Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 12:06:24 +0100 Subject: [PATCH 01/14] feat(processes): start processes on domain events in the same transaction --- docs/architecture.md | 8 +- packages/api/drizzle/0038_processes.sql | 4 +- packages/api/drizzle/meta/0038_snapshot.json | 12 +- packages/api/importRules.json | 2 + packages/api/src/core/db/beforeCommitBus.ts | 30 ++ packages/api/src/core/db/index.ts | 4 +- .../api/src/core/db/transaction.int.test.ts | 144 +++++++- packages/api/src/core/db/transaction.ts | 52 ++- packages/api/src/core/events/eventBus.ts | 3 + packages/api/src/core/events/index.ts | 9 +- packages/api/src/core/moduleManifest.ts | 4 +- .../core/records/app/recordHistoryService.ts | 4 +- .../src/core/records/domain/recordHistory.ts | 3 +- packages/api/src/core/server.ts | 8 +- .../employees/app/employeeDirectory.ts | 12 +- .../modules/processes/app/dueRunService.ts | 41 +-- .../modules/processes/app/eventRunService.ts | 191 ++++++++++ .../modules/processes/app/processService.ts | 38 +- .../src/modules/processes/app/runReader.ts | 1 + .../src/modules/processes/app/runStarter.ts | 341 +++++++++--------- .../app/startRunsOnEvents.int.test.ts | 340 +++++++++++++++++ .../modules/processes/app/workingCalendars.ts | 76 ++++ .../api/src/modules/processes/db/index.ts | 7 +- .../modules/processes/db/processRepository.ts | 122 +++++-- .../modules/processes/db/processRunsTable.ts | 9 +- .../processes/domain/eventTrigger.test.ts | 107 ++++++ .../modules/processes/domain/eventTrigger.ts | 73 ++++ .../api/src/modules/processes/domain/index.ts | 11 + .../domain/processDefinition.test.ts | 80 +++- .../processes/domain/processDefinition.ts | 73 +++- .../modules/processes/domain/processRun.ts | 9 +- packages/api/src/modules/processes/index.ts | 3 + packages/api/src/modules/processes/module.md | 5 +- .../modules/tasks/app/createTask.int.test.ts | 85 ++++- .../api/src/modules/tasks/app/createTasks.ts | 95 +++++ packages/api/src/modules/tasks/app/index.ts | 3 +- .../modules/tasks/app/markDone.int.test.ts | 27 +- .../src/modules/tasks/app/registrations.ts | 23 +- .../api/src/modules/tasks/app/taskService.ts | 86 +---- .../src/modules/tasks/db/taskRepository.ts | 10 +- packages/api/test/support/domainEvents.ts | 20 + packages/api/test/support/index.ts | 3 +- packages/api/test/support/processService.ts | 30 +- packages/api/test/support/seedProcess.ts | 28 ++ packages/api/test/support/seedTask.ts | 37 +- packages/api/test/support/services.ts | 4 +- packages/api/test/support/taskService.ts | 11 + packages/api/test/support/testSeed.ts | 4 + 48 files changed, 1903 insertions(+), 389 deletions(-) create mode 100644 packages/api/src/core/db/beforeCommitBus.ts create mode 100644 packages/api/src/modules/processes/app/eventRunService.ts create mode 100644 packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts create mode 100644 packages/api/src/modules/processes/app/workingCalendars.ts create mode 100644 packages/api/src/modules/processes/domain/eventTrigger.test.ts create mode 100644 packages/api/src/modules/processes/domain/eventTrigger.ts create mode 100644 packages/api/src/modules/tasks/app/createTasks.ts create mode 100644 packages/api/test/support/domainEvents.ts create mode 100644 packages/api/test/support/seedProcess.ts create mode 100644 packages/api/test/support/taskService.ts diff --git a/docs/architecture.md b/docs/architecture.md index 9f60e2baf..1a630ca33 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -79,10 +79,10 @@ For example, a domain rule decides whether an import source may retry. The appli - **Domain changes notify through events**, never injected callbacks or callback ports ([RULE72](code-quality.md#layers-and-place)). The publisher owns the fact and each receiving module owns its reaction, so adding a reaction does not add dependencies to the publisher or its callers. Work that must commit together uses direct public operations in one transaction. - **Postgres is the durable work store.** pg-boss runs background jobs in the API process, using the instance's database. Enqueuing joins the business transaction, so rollback drops the work. Handlers run as the system, are idempotent and reject stale work; retries and terminal failures belong to the job lifecycle. - **Scheduled jobs** are pg-boss schedules a module declares beside its job handler (a cron and a time zone); a missed occurrence runs once on the next start. -- **Events currently run in process after commit.** Rollback drops them. Handlers run sequentially and are awaited; handler failures are logged without undoing the committed operation. A crash between commit and dispatch can lose an event, an accepted limitation for now. -- **Record changes publish `records.changed`**, one event per history line with the record, the operation, the changed field keys and the cause, never values. Processes will be triggered by it; running many changes as one batch belongs to the future process runner, not the event. -- **The durability decision is a transactional outbox**, written with the business change and audit record and drained by pg-boss. Delivery is ordered per entity and consumers remain idempotent. This avoids an external queue or a second workflow engine; the outbox is not implemented yet. -- **Workflows** such as approvals and onboarding are domain state machines, with durable jobs executing their steps. A process starts from a trigger on a record, never from a feature calling it: a record's date starts it by a daily job today, and record changes will through the outbox. A step's task is finished through the process, which changes the task and the run in one transaction. +- **Events reach handlers at two moments.** Before the commit, a module's before-commit handler receives all of a transaction's new events at once, inside it, round after round while handlers publish more (at most ten rounds); its failure rolls the transaction back, so its reaction commits with the change or not at all. After the commit, in-process handlers run sequentially and are awaited; their failures are logged without undoing the committed operation, and a crash between commit and dispatch can lose an event, an accepted limitation for them. Rollback drops both. +- **Record changes publish `records.changed`**, one event per history line with the record, the operation, the changed field keys and the cause, never values. It notifies; processes trigger on domain events instead. +- **Durable delivery after the commit is a transactional outbox**, a pg-boss job queued with the business change, for reactions that are slow or call outside systems. It is not implemented yet; this avoids an external queue or a second workflow engine. +- **Workflows** such as approvals and onboarding are domain state machines, with durable jobs executing their steps. A process starts from a trigger, never from a feature calling it: a record's date starts it by a daily job, and a domain event a module lists as triggering (a person added, a task created or its status changed) starts it in the transaction that published the event, one run per event and record, batched per process like a bulk save. A run that cannot start fails that change. A step's task is finished through the process, which changes the task and the run in one transaction. - **A process acts as the system on its own and as the person when they act.** A run's own writes, such as creating its tasks or cancelling itself, run as the system with the run as their history cause (`PROCESS_RUN`); an assignee marking their step done, or an admin starting or cancelling a run, acts as themselves under their own permissions. Who acts for a step that calls another module is not decided yet. - **A run notices removals when it next runs.** Nothing reacts to a person's removal: a step still waiting on a task whose assignee was removed, or a removed subject, cancels the run the next time a command or the daily job touches it, and the cancellation commits instead of being thrown. diff --git a/packages/api/drizzle/0038_processes.sql b/packages/api/drizzle/0038_processes.sql index dc9e41a65..2fc43a446 100644 --- a/packages/api/drizzle/0038_processes.sql +++ b/packages/api/drizzle/0038_processes.sql @@ -55,6 +55,7 @@ CREATE TABLE "process_runs" ( "started_by_employee_id" uuid, "trigger_date" date, "start_key" text, + "trigger_event" jsonb, "start_day" date NOT NULL, "status" text NOT NULL, "finished_at" timestamp with time zone, @@ -65,7 +66,8 @@ CREATE TABLE "process_runs" ( "created_at" timestamp with time zone DEFAULT now() NOT NULL, "updated_at" timestamp with time zone DEFAULT now() NOT NULL, CONSTRAINT "process_runs_status_known" CHECK ("process_runs"."status" in ('in_progress', 'finished', 'cancelled')), - CONSTRAINT "process_runs_start_cause_known" CHECK ("process_runs"."start_cause" in ('started_on_record_date', 'started_by_admin')), + CONSTRAINT "process_runs_start_cause_known" CHECK ("process_runs"."start_cause" in ('started_on_record_date', 'started_by_admin', 'started_on_event')), + CONSTRAINT "process_runs_trigger_event_follows_start_cause" CHECK (("process_runs"."start_cause" = 'started_on_event') = ("process_runs"."trigger_event" is not null)), CONSTRAINT "process_runs_cancel_cause_known" CHECK ("process_runs"."cancel_cause" in ('cancelled_by_admin', 'cancelled_because_step_assignee_was_removed', 'cancelled_because_subject_was_removed')), CONSTRAINT "process_runs_columns_follow_status" CHECK (("process_runs"."status" = 'finished') = ("process_runs"."finished_at" is not null) and ("process_runs"."status" = 'cancelled') = ("process_runs"."cancelled_at" is not null) diff --git a/packages/api/drizzle/meta/0038_snapshot.json b/packages/api/drizzle/meta/0038_snapshot.json index 3c2c9d6ab..3de22da07 100644 --- a/packages/api/drizzle/meta/0038_snapshot.json +++ b/packages/api/drizzle/meta/0038_snapshot.json @@ -3595,6 +3595,12 @@ "primaryKey": false, "notNull": false }, + "trigger_event": { + "name": "trigger_event", + "type": "jsonb", + "primaryKey": false, + "notNull": false + }, "start_day": { "name": "start_day", "type": "date", @@ -3797,7 +3803,11 @@ }, "process_runs_start_cause_known": { "name": "process_runs_start_cause_known", - "value": "\"process_runs\".\"start_cause\" in ('started_on_record_date', 'started_by_admin')" + "value": "\"process_runs\".\"start_cause\" in ('started_on_record_date', 'started_by_admin', 'started_on_event')" + }, + "process_runs_trigger_event_follows_start_cause": { + "name": "process_runs_trigger_event_follows_start_cause", + "value": "(\"process_runs\".\"start_cause\" = 'started_on_event') = (\"process_runs\".\"trigger_event\" is not null)" }, "process_runs_cancel_cause_known": { "name": "process_runs_cancel_cause_known", diff --git a/packages/api/importRules.json b/packages/api/importRules.json index b3573373d..9447198b0 100644 --- a/packages/api/importRules.json +++ b/packages/api/importRules.json @@ -173,6 +173,8 @@ "src/modules/employees/db", "src/modules/imports/db", "src/modules/imports/domain", + "src/modules/processes/db", + "src/modules/processes/domain", "src/modules/slack/db", "src/modules/tasks/db", "src/modules/tasks/domain", diff --git a/packages/api/src/core/db/beforeCommitBus.ts b/packages/api/src/core/db/beforeCommitBus.ts new file mode 100644 index 000000000..5a71567d1 --- /dev/null +++ b/packages/api/src/core/db/beforeCommitBus.ts @@ -0,0 +1,30 @@ +import type { PublishedEvent } from "../events"; +import type { Tx } from "./transaction"; + +/** Reacts to a transaction's events inside it, before it commits; an error rolls it back. */ +export type BeforeCommitHandler = (tx: Tx, events: readonly PublishedEvent[]) => Promise; + +/** + * Hands each transaction's events to the modules that must react in the same transaction, such as + * processes starting their runs, so the reaction commits with the change or not at all. + */ +export interface BeforeCommitBus { + subscribe(handler: BeforeCommitHandler): void; + /** Runs each subscribed handler in turn on the events; the first failure is thrown. */ + deliver(tx: Tx, events: readonly PublishedEvent[]): Promise; +} + +export function createBeforeCommitBus(): BeforeCommitBus { + const handlers: BeforeCommitHandler[] = []; + return { + subscribe(handler) { + handlers.push(handler); + }, + + async deliver(tx, events) { + for (const handler of handlers) { + await handler(tx, events); + } + }, + }; +} diff --git a/packages/api/src/core/db/index.ts b/packages/api/src/core/db/index.ts index 7005d15c4..b1cc9140a 100644 --- a/packages/api/src/core/db/index.ts +++ b/packages/api/src/core/db/index.ts @@ -5,10 +5,12 @@ export { MIGRATION_LOCK_KEY } from "./migrationRun"; export { anyOf } from "./anyOf"; export { jsonb } from "./jsonb"; export { afterKeyset, keysetOrder, nullsLast, orderByKeyset } from "./keyset"; -export { ormOf, transactor } from "./transaction"; +export { createBeforeCommitBus } from "./beforeCommitBus"; +export { MAX_BEFORE_COMMIT_ROUNDS, ormOf, transactor } from "./transaction"; export { isForeignKeyViolation, isUniqueViolation } from "./uniqueViolation"; // Public types. +export type { BeforeCommitBus, BeforeCommitHandler } from "./beforeCommitBus"; export type { Database, Orm, OrmTransaction } from "./database"; export type { KeysetOrder } from "./keyset"; export type { Transact, Tx } from "./transaction"; diff --git a/packages/api/src/core/db/transaction.int.test.ts b/packages/api/src/core/db/transaction.int.test.ts index eadd58936..ecbed6bd2 100644 --- a/packages/api/src/core/db/transaction.int.test.ts +++ b/packages/api/src/core/db/transaction.int.test.ts @@ -1,8 +1,8 @@ import { EXAMPLE_PAYLOAD, expect, SYSTEM, test, instances, setupSteps } from "#test/support"; -import { createEventBus } from "../events"; +import { createEventBus, type PublishedEvent } from "../events"; import { createLogger } from "../logger"; -import { transactor } from "."; +import { createBeforeCommitBus, MAX_BEFORE_COMMIT_ROUNDS, transactor } from "."; test("dispatches queued events after the write commits", async ({ orm, seed }) => { await seed.owner(); @@ -90,3 +90,143 @@ test("refuses the publisher's transaction inside an after-commit handler", async expect(rejection).toEqual(new Error("The transaction has ended")); expect((await instances.readInstance(orm)).step).toBe("company"); }); + +test("hands identified events to handlers before committing their writes with the work", async ({ + orm, + seed, +}) => { + await seed.owner(); + await seed.assignment(); + const beforeCommit = createBeforeCommitBus(); + const transact = transactor(orm, createEventBus(createLogger("silent")), beforeCommit); + const received: PublishedEvent[] = []; + beforeCommit.subscribe(async (tx, events) => { + received.push(...events); + expect((await instances.readInstance(orm)).step).toBe("company"); + expect((await instances.readInstance(tx)).step).toBe("import"); + await setupSteps.approveImportStep(SYSTEM, tx); + }); + + await transact(async (tx) => { + await setupSteps.completeCompanyStep(SYSTEM, tx); + tx.publish("example.happened", EXAMPLE_PAYLOAD); + tx.publish("example.happened", { id: "example-2" }); + }); + + expect(received).toEqual([ + { id: expect.any(String) as unknown, type: "example.happened", payload: EXAMPLE_PAYLOAD }, + { id: expect.any(String) as unknown, type: "example.happened", payload: { id: "example-2" } }, + ]); + expect(received[0]!.id).toMatch(/^[0-9a-f]{8}-(?:[0-9a-f]{4}-){3}[0-9a-f]{12}$/); + expect(received[1]!.id).not.toBe(received[0]!.id); + expect((await instances.readInstance(orm)).step).toBe("ready"); +}); + +test("delivers handler-published events in the next round and after the commit", async ({ + orm, + seed, +}) => { + await seed.owner(); + const eventBus = createEventBus(createLogger("silent")); + const beforeCommit = createBeforeCommitBus(); + const transact = transactor(orm, eventBus, beforeCommit); + const rounds: (readonly PublishedEvent[])[] = []; + const received: unknown[] = []; + beforeCommit.subscribe((tx, events) => { + rounds.push(events); + expect(received).toEqual([]); + if ( + events.some( + (event) => event.type === "example.happened" && event.payload.id === EXAMPLE_PAYLOAD.id, + ) + ) { + tx.publish("example.happened", { id: "from-handler" }); + } + return Promise.resolve(); + }); + eventBus.subscribe({ + "example.happened": async (payload) => { + expect((await instances.readInstance(orm)).step).toBe("import"); + received.push(payload); + }, + }); + + await transact(async (tx) => { + await setupSteps.completeCompanyStep(SYSTEM, tx); + tx.publish("example.happened", EXAMPLE_PAYLOAD); + }); + + expect(rounds).toMatchObject([ + [{ type: "example.happened", payload: EXAMPLE_PAYLOAD }], + [{ type: "example.happened", payload: { id: "from-handler" } }], + ]); + expect(received).toEqual([EXAMPLE_PAYLOAD, { id: "from-handler" }]); +}); + +test("rolls back the work and delivers no events when a before-commit handler fails", async ({ + orm, + seed, +}) => { + await seed.owner(); + const eventBus = createEventBus(createLogger("silent")); + const beforeCommit = createBeforeCommitBus(); + const transact = transactor(orm, eventBus, beforeCommit); + const received: unknown[] = []; + const failure = new Error("Cannot start the work"); + eventBus.subscribe({ + "example.happened": (payload) => { + received.push(payload); + return Promise.resolve(); + }, + }); + beforeCommit.subscribe(async (tx) => { + await setupSteps.approveImportStep(SYSTEM, tx); + tx.publish("example.happened", { id: "from-handler" }); + throw failure; + }); + + await expect( + transact(async (tx) => { + await setupSteps.completeCompanyStep(SYSTEM, tx); + tx.publish("example.happened", EXAMPLE_PAYLOAD); + }), + ).rejects.toBe(failure); + + expect((await instances.readInstance(orm)).step).toBe("company"); + expect(received).toEqual([]); +}); + +test("rolls back events that keep publishing after ten before-commit rounds", async ({ + orm, + seed, +}) => { + await seed.owner(); + const eventBus = createEventBus(createLogger("silent")); + const beforeCommit = createBeforeCommitBus(); + const transact = transactor(orm, eventBus, beforeCommit); + const received: unknown[] = []; + const rounds: (readonly PublishedEvent[])[] = []; + eventBus.subscribe({ + "example.happened": (payload) => { + received.push(payload); + return Promise.resolve(); + }, + }); + beforeCommit.subscribe((tx, events) => { + rounds.push(events); + tx.publish("example.happened", EXAMPLE_PAYLOAD); + return Promise.resolve(); + }); + + await expect( + transact(async (tx) => { + await setupSteps.completeCompanyStep(SYSTEM, tx); + tx.publish("example.happened", EXAMPLE_PAYLOAD); + }), + ).rejects.toThrow("Events kept publishing more events before the commit after 10 rounds."); + + expect(rounds).toHaveLength(MAX_BEFORE_COMMIT_ROUNDS); + expect(rounds).toHaveLength(10); + expect((await instances.readInstance(orm)).step).toBe("company"); + expect(received).toEqual([]); +}); diff --git a/packages/api/src/core/db/transaction.ts b/packages/api/src/core/db/transaction.ts index abdf1176a..9654d7073 100644 --- a/packages/api/src/core/db/transaction.ts +++ b/packages/api/src/core/db/transaction.ts @@ -1,5 +1,8 @@ +import { randomUUID } from "node:crypto"; + import { invalidInput } from "../applicationError"; -import type { DomainEvents, EventBus, EventType, DomainEvent } from "../events"; +import type { DomainEvents, EventBus, EventType, PublishedEvent } from "../events"; +import { type BeforeCommitBus, createBeforeCommitBus } from "./beforeCommitBus"; import type { Orm, OrmTransaction } from "./database"; import { isUnstorableText } from "./uniqueViolation"; @@ -8,12 +11,12 @@ import { isUnstorableText } from "./uniqueViolation"; * it. Only database adapters reach the ORM behind it, through `ormOf`. */ export interface Tx { - /** Queues an event; it is dispatched once the transaction commits. */ + /** Queues an event: handed to before-commit handlers inside the transaction, dispatched after it commits. */ publish(type: Type, payload: DomainEvents[Type]): void; } class Transaction implements Tx { - readonly events: DomainEvent[] = []; + readonly events: PublishedEvent[] = []; #orm: OrmTransaction | undefined; constructor(orm: OrmTransaction) { @@ -31,7 +34,7 @@ class Transaction implements Tx { // Reading `orm` checks the transaction is still open. void this.orm; // TypeScript cannot tie a generic `type` to its payload in the union, hence the cast. - this.events.push({ type, payload } as DomainEvent); + this.events.push({ id: randomUUID(), type, payload } as PublishedEvent); } end(): void { @@ -42,31 +45,41 @@ class Transaction implements Tx { /** Runs work in one transaction, as `runInTransaction` describes. */ export type Transact = (work: (tx: Tx) => Promise) => Promise; -/** Binds `runInTransaction` to the database and the event bus, once at startup. */ -export function transactor(orm: Orm, eventBus: EventBus): Transact { - return (work) => runInTransaction(orm, eventBus, work); +/** Binds `runInTransaction` to the database and the event buses, once at startup. */ +export function transactor( + orm: Orm, + eventBus: EventBus, + beforeCommit: BeforeCommitBus = createBeforeCommitBus(), +): Transact { + return (work) => runInTransaction(orm, { eventBus, beforeCommit }, work); } +/** How many rounds of events before a commit may publish more: a loop of reactions beyond it is a bug. */ +export const MAX_BEFORE_COMMIT_ROUNDS = 10; + /** The ORM to query: the pool, or the open transaction behind `db`. */ export function ormOf(db: Orm | Tx): Orm | OrmTransaction { return db instanceof Transaction ? db.orm : (db as Orm); } /** - * Runs `work` in one transaction. After the commit, dispatches the events published - * on it; a rollback drops them. Inner operations join `tx`; there is no nesting. + * Runs `work` in one transaction. Before the commit, hands the events published on it to the + * before-commit handlers, round after round while they publish more; after the commit, dispatches + * them all. A rollback drops them. Inner operations join `tx`; there is no nesting. */ async function runInTransaction( orm: Orm, - eventBus: EventBus, + { eventBus, beforeCommit }: { eventBus: EventBus; beforeCommit: BeforeCommitBus }, work: (tx: Tx) => Promise, ): Promise { let transaction: Transaction | undefined; let result: T; try { - result = await orm.transaction((ormTransaction) => { + result = await orm.transaction(async (ormTransaction) => { transaction = new Transaction(ormTransaction); - return work(transaction); + const value = await work(transaction); + await handBeforeCommit(transaction, beforeCommit); + return value; }); } catch (error) { // Text Postgres cannot store, such as a NUL in a name, is input the caller must change. @@ -78,3 +91,18 @@ async function runInTransaction( await eventBus.dispatch(transaction!.events); return result; } + +/** Hands each round's new events to the before-commit handlers until a round publishes none. */ +async function handBeforeCommit(transaction: Transaction, beforeCommit: BeforeCommitBus) { + let handed = 0; + for (let round = 0; handed < transaction.events.length; round += 1) { + if (round === MAX_BEFORE_COMMIT_ROUNDS) { + throw new Error( + `Events kept publishing more events before the commit after ${MAX_BEFORE_COMMIT_ROUNDS} rounds.`, + ); + } + const events = transaction.events.slice(handed); + handed = transaction.events.length; + await beforeCommit.deliver(transaction, events); + } +} diff --git a/packages/api/src/core/events/eventBus.ts b/packages/api/src/core/events/eventBus.ts index f46d78745..581d84a09 100644 --- a/packages/api/src/core/events/eventBus.ts +++ b/packages/api/src/core/events/eventBus.ts @@ -13,6 +13,9 @@ export type DomainEvent = { [Type in EventType]: { type: Type; payload: DomainEvents[Type] }; }[EventType]; +/** An event with the id it got when it was published, which stays with it wherever it goes. */ +export type PublishedEvent = DomainEvent & { id: string }; + /** What a module does when an event happens, by event type. */ export type EventHandlers = { [Type in EventType]?: (payload: DomainEvents[Type]) => Promise; diff --git a/packages/api/src/core/events/index.ts b/packages/api/src/core/events/index.ts index ff4c93dbb..a23c4c253 100644 --- a/packages/api/src/core/events/index.ts +++ b/packages/api/src/core/events/index.ts @@ -2,4 +2,11 @@ export { createEventBus } from "./eventBus"; // Public types. -export type { DomainEvents, EventBus, EventHandlers, EventType, DomainEvent } from "./eventBus"; +export type { + DomainEvents, + EventBus, + EventHandlers, + EventType, + DomainEvent, + PublishedEvent, +} from "./eventBus"; diff --git a/packages/api/src/core/moduleManifest.ts b/packages/api/src/core/moduleManifest.ts index 4561cbc54..e2f3f8656 100644 --- a/packages/api/src/core/moduleManifest.ts +++ b/packages/api/src/core/moduleManifest.ts @@ -3,7 +3,7 @@ import type express from "express"; import type { AgentSecurityResolver } from "./agentAccess"; -import type { Orm, Transact } from "./db"; +import type { BeforeCommitHandler, Orm, Transact } from "./db"; import type { AddApprovedPeople, PasswordResetEmailService, SetupDemo } from "./instance"; import type { EventHandlers } from "./events"; import type { JobHandlers, JobQueue } from "./jobs"; @@ -39,6 +39,8 @@ export interface ModuleManifest { resolvers: Resolvers; /** What the module does when another module's event happens. */ events?: EventHandlers; + /** What the module does with a transaction's events inside it, committing with them or not at all. */ + eventsBeforeCommit?: BeforeCommitHandler; /** The record types the module owns that keep history: who may read it and how to load them. */ recordTypes?: RecordTypeRegistry; /** The module's background jobs, run as the system. */ diff --git a/packages/api/src/core/records/app/recordHistoryService.ts b/packages/api/src/core/records/app/recordHistoryService.ts index ee9674af7..87bf1c22b 100644 --- a/packages/api/src/core/records/app/recordHistoryService.ts +++ b/packages/api/src/core/records/app/recordHistoryService.ts @@ -57,12 +57,12 @@ export function createRecordHistoryService(): RecordHistoryService { async append(ctx, tx, { recordType, operation, cause, reason = null, records }) { const author = authorOf(ctx); const lines = await recordHistoryRepository(tx).insert( - records.map(({ recordId, changes }) => ({ + records.map(({ recordId, changes, cause: own = cause }) => ({ recordType, recordId, operation, author, - cause: { kind: cause.kind, ref: cause.ref ?? null }, + cause: { kind: own.kind, ref: own.ref ?? null }, reason, changes, })), diff --git a/packages/api/src/core/records/domain/recordHistory.ts b/packages/api/src/core/records/domain/recordHistory.ts index 2f12a4d66..a5ea8c06d 100644 --- a/packages/api/src/core/records/domain/recordHistory.ts +++ b/packages/api/src/core/records/domain/recordHistory.ts @@ -68,7 +68,8 @@ export interface HistoryAppend { operation: RecordOperation; cause: { kind: CauseKind; ref?: string | null }; reason?: string | null; - records: { recordId: string; changes: FieldChange[] }[]; + /** A record's own `cause` replaces the append's, such as tasks of different runs in one batch. */ + records: { recordId: string; changes: FieldChange[]; cause?: HistoryAppend["cause"] }[]; } /** The author of a change made with `ctx`. */ diff --git a/packages/api/src/core/server.ts b/packages/api/src/core/server.ts index ef73fdf97..a97102d5b 100644 --- a/packages/api/src/core/server.ts +++ b/packages/api/src/core/server.ts @@ -14,7 +14,7 @@ import { createAgentConnectionService, } from "./agentAccess"; import { authHandler, type DeploymentServer } from "./auth"; -import { connectDatabase, type Database, transactor } from "./db"; +import { connectDatabase, createBeforeCommitBus, type Database, transactor } from "./db"; import type { Env } from "./env"; import { createEventBus } from "./events"; import { type GraphqlApi, startGraphqlApi } from "./graphql"; @@ -58,7 +58,8 @@ export async function startServer( const secretBox = createSecretBox(await ensureSecret(env.dataDir, "encryption-key")); const database = connectDatabase(env.databaseUrl, logger); const eventBus = createEventBus(logger); - const transact = transactor(database.orm, eventBus); + const beforeCommit = createBeforeCommitBus(); + const transact = transactor(database.orm, eventBus, beforeCommit); const jobRunner = createJobRunner(env.databaseUrl, logger); const readiness = new Readiness(() => database.checkConnection()); const app = express(); @@ -132,6 +133,9 @@ export async function startServer( if (module.events) { eventBus.subscribe(module.events); } + if (module.eventsBeforeCommit) { + beforeCommit.subscribe(module.eventsBeforeCommit); + } if (module.router) { app.use( "/api", diff --git a/packages/api/src/modules/employees/app/employeeDirectory.ts b/packages/api/src/modules/employees/app/employeeDirectory.ts index 580a8fcb1..446334898 100644 --- a/packages/api/src/modules/employees/app/employeeDirectory.ts +++ b/packages/api/src/modules/employees/app/employeeDirectory.ts @@ -1,4 +1,4 @@ -import type { Orm } from "../../../core/db"; +import type { Orm, Tx } from "../../../core/db"; import { createOwnerService } from "../../../core/instance"; import type { EmployeeDirectory } from "../../../core/security"; import { todayInUtc } from "../../../utils"; @@ -6,10 +6,10 @@ import { employeeRepository, employmentRepository } from "../db"; const owners = createOwnerService(); -/** What permission checks read about people: who a user is, and who is above whom today. */ -export function employeeDirectory(orm: Orm): EmployeeDirectory { - const employees = employeeRepository(orm); - const employments = employmentRepository(orm); +/** What permission checks read about people: who a user is, and who is above whom today; in `db`. */ +export function employeeDirectory(db: Orm | Tx): EmployeeDirectory { + const employees = employeeRepository(db); + const employments = employmentRepository(db); const employeeIdOfUser = async (userId: string) => (await employees.findByUserId(userId))?.id ?? null; return { @@ -18,7 +18,7 @@ export function employeeDirectory(orm: Orm): EmployeeDirectory { managersAbove: (employeeIds) => employments.findManagersAbove(employeeIds, todayInUtc()), reportsBelow: (employeeId) => employments.findReportsBelow(employeeId, todayInUtc()), ownersEmployeeId: async () => { - const owner = await owners.readOwner(orm); + const owner = await owners.readOwner(db); return owner ? employeeIdOfUser(owner.userId) : null; }, }; diff --git a/packages/api/src/modules/processes/app/dueRunService.ts b/packages/api/src/modules/processes/app/dueRunService.ts index 3acdc2f4f..98c49d546 100644 --- a/packages/api/src/modules/processes/app/dueRunService.ts +++ b/packages/api/src/modules/processes/app/dueRunService.ts @@ -39,12 +39,13 @@ export function createDueRunService({ for (const process of await processRepository(orm).listProcesses()) { if (!process.turnedOnAt) continue; const { activeVersion } = process; - const definition = parseProcessDefinition( + const { trigger } = parseProcessDefinition( activeVersion.schemaVersion, activeVersion.definition, ); + if (trigger.kind !== "on_record_date") continue; const turnedOnDay = dateInLondon(process.turnedOnAt); - const due = await transact((tx) => starter.findDue(tx, definition, { today, turnedOnDay })); + const due = await transact((tx) => starter.findDue(tx, trigger, { today, turnedOnDay })); const withRuns = await processRepository(orm).findRecordsWithRuns( process.id, "employee", @@ -84,14 +85,8 @@ async function startOne( today: CalendarDate, ): Promise { try { - const workspaceOwnerEmployeeId = await starter.readWorkspaceOwner(); const run = await transact((tx) => - startIfStillDue( - starter, - tx, - { processId: process.id, employeeId, today }, - workspaceOwnerEmployeeId, - ), + startIfStillDue(starter, tx, { processId: process.id, employeeId, today }), ); if (run) { logger.info( @@ -117,7 +112,6 @@ async function startIfStillDue( starter: RunStarter, tx: Tx, { processId, employeeId, today }: { processId: string; employeeId: string; today: CalendarDate }, - workspaceOwnerEmployeeId: string | null, ): Promise { const processes = processRepository(tx); await processes.lockProcess(processId); @@ -126,6 +120,7 @@ async function startIfStillDue( if (!process?.turnedOnAt || !subject?.startDate) return null; const { activeVersion } = process; const definition = parseProcessDefinition(activeVersion.schemaVersion, activeVersion.definition); + if (definition.trigger.kind !== "on_record_date") return null; const triggerDate = dueTriggerDate(definition.trigger, subject.startDate, { today, turnedOnDay: dateInLondon(process.turnedOnAt), @@ -133,17 +128,21 @@ async function startIfStillDue( const hasRun = (await processes.findRecordsWithRuns(processId, "employee", [employeeId])).size > 0; if (!triggerDate || hasRun) return null; - return starter.start(tx, { - process, - definition, - subject, - startCause: "started_on_record_date", - startedByEmployeeId: null, - triggerDate, - startKey: `on_record_date:employee:${employeeId}:${triggerDate}`, - startDay: today, - workspaceOwnerEmployeeId, - }); + const [run] = await starter.startMany(tx, [ + { + process, + definition, + record: { recordType: "employee", recordId: employeeId }, + subject, + startCause: "started_on_record_date", + startedByEmployeeId: null, + triggerDate, + triggerEvent: null, + startKey: `on_record_date:employee:${employeeId}:${triggerDate}`, + startDay: today, + }, + ]); + return run!; } function errorKind(error: unknown): string | undefined { diff --git a/packages/api/src/modules/processes/app/eventRunService.ts b/packages/api/src/modules/processes/app/eventRunService.ts new file mode 100644 index 000000000..48d4a6548 --- /dev/null +++ b/packages/api/src/modules/processes/app/eventRunService.ts @@ -0,0 +1,191 @@ +import type { Tx } from "../../../core/db"; +import type { PublishedEvent } from "../../../core/events"; +import type { Logger } from "../../../core/logger"; +import type { SecurityContext } from "../../../core/security"; +import type { CalendarDate } from "../../../utils"; +import type { TaskService } from "../../tasks"; +import { processRepository, type StoredProcess } from "../db"; +import { + type EventStart, + isTriggeringEvent, + matchEventStarts, + parseProcessDefinition, + type ProcessDefinition, +} from "../domain"; +import type { RunStarter, RunSubject } from "./runStarter"; + +/** Starts the runs that a transaction's events trigger, inside that transaction. */ +export interface EventRunService { + /** + * Trusted, as the system, before the commit: one run per turned-on process and record each event + * triggers, skipping a record whose run of the process is in progress or that is gone. An error + * rolls the transaction back. + */ + startRunsOnEvents(ctx: SecurityContext, tx: Tx, events: readonly PublishedEvent[]): Promise; +} + +export function createEventRunService(dependencies: EventRunDependencies): EventRunService { + return { + async startRunsOnEvents(ctx, tx, events) { + ctx.requireSystem(); + const triggering = events.filter(({ type }) => isTriggeringEvent(type)); + if (triggering.length === 0) return; + const processes = await processRepository(tx).lockProcessesStartedBy([ + ...new Set(triggering.map(({ type }) => type)), + ]); + const definitions = new Map( + processes.map(({ id, activeVersion }) => [ + id, + parseProcessDefinition(activeVersion.schemaVersion, activeVersion.definition), + ]), + ); + const starts = matchEventStarts( + processes.map(({ id }) => ({ processId: id, trigger: definitions.get(id)!.trigger })), + triggering, + ); + for (const process of processes) { + const own = starts.filter(({ processId }) => processId === process.id); + if (own.length === 0) continue; + await startProcessRuns(dependencies, tx, { + process, + definition: definitions.get(process.id)!, + starts: own, + }); + } + }, + }; +} + +interface EventRunDependencies { + logger: Logger; + tasks: TaskService; + starter: RunStarter; + /** Today in London: the start day of the runs. */ + today: () => CalendarDate; +} + +type Start = EventStart; + +/** + * Starts one process's runs in waves, each record at most once a wave, so a second event for a + * record finds the first one's run when it is still in progress. + */ +async function startProcessRuns( + { logger, tasks, starter, today }: EventRunDependencies, + tx: Tx, + { + process, + definition, + starts, + }: { process: StoredProcess; definition: ProcessDefinition; starts: readonly Start[] }, +): Promise { + const { recordType } = definition.subject; + const subjects = await readRecordSubjects({ tasks, starter }, tx, recordType, starts); + const runs = processRepository(tx); + for (const wave of wavesOf(starts)) { + const inProgress = await runs.findRecordsWithRunInProgress( + process.id, + recordType, + wave.map(({ recordId }) => recordId), + ); + const ready = wave.filter((start) => { + const reason = skipReasonOf(start, { subjects, inProgress }); + if (reason) logSkip(logger, process, start, reason); + return reason === null; + }); + const started = await starter.startMany( + tx, + ready.map(({ event, recordId }) => ({ + process, + definition, + record: { recordType, recordId }, + subject: subjects.get(recordId) ?? null, + startCause: "started_on_event", + startedByEmployeeId: null, + triggerDate: null, + triggerEvent: event, + startKey: `on_event:${event.id}:${recordId}`, + startDay: today(), + })), + ); + if (started.length > 0) logStarted(logger, process, started, ready); + } +} + +/** Why a start is skipped: its record is gone or already has a run of the process in progress. */ +function skipReasonOf( + { recordId }: Start, + { + subjects, + inProgress, + }: { subjects: ReadonlyMap; inProgress: ReadonlySet }, +): "record_gone" | "run_in_progress" | null { + if (!subjects.has(recordId)) return "record_gone"; + return inProgress.has(recordId) ? "run_in_progress" : null; +} + +/** + * The subject of each record that still exists, by record id: the employee itself, or a task's + * subject, null for a task about nobody. + */ +async function readRecordSubjects( + { tasks, starter }: Pick, + tx: Tx, + recordType: ProcessDefinition["subject"]["recordType"], + starts: readonly Start[], +): Promise> { + const recordIds = [...new Set(starts.map(({ recordId }) => recordId))]; + if (recordType === "employee") return starter.readSubjects(tx, recordIds); + const found = await tasks.readTasksInTransaction(tx, recordIds); + const subjects = await starter.readSubjects( + tx, + found.flatMap(({ subjectEmployeeId }) => (subjectEmployeeId ? [subjectEmployeeId] : [])), + ); + return new Map( + found.map(({ id, subjectEmployeeId }) => [ + id, + subjectEmployeeId ? (subjects.get(subjectEmployeeId) ?? null) : null, + ]), + ); +} + +/** The starts split so each record appears at most once a wave, keeping their order. */ +function wavesOf(starts: readonly Start[]): Start[][] { + const waves: Start[][] = []; + for (const start of starts) { + const wave = waves.find((candidate) => + candidate.every(({ recordId }) => recordId !== start.recordId), + ); + if (wave) wave.push(start); + else waves.push([start]); + } + return waves; +} + +function logSkip( + logger: Logger, + process: StoredProcess, + { event, recordId }: Start, + reason: "record_gone" | "run_in_progress", +): void { + logger.info( + { processKey: process.key, eventId: event.id, eventType: event.type, recordId, reason }, + "Process run not started for an event", + ); +} + +function logStarted( + logger: Logger, + process: StoredProcess, + runs: readonly { id: string }[], + starts: readonly Start[], +): void { + logger.info( + { + processKey: process.key, + runIds: runs.map(({ id }) => id), + eventIds: [...new Set(starts.map(({ event }) => event.id))], + }, + "Process runs started with the change that triggered them", + ); +} diff --git a/packages/api/src/modules/processes/app/processService.ts b/packages/api/src/modules/processes/app/processService.ts index 9daa25511..3c28493aa 100644 --- a/packages/api/src/modules/processes/app/processService.ts +++ b/packages/api/src/modules/processes/app/processService.ts @@ -14,6 +14,7 @@ import type { TaskService } from "../../tasks"; import { processRepository, type StoredProcess } from "../db"; import { parseProcessDefinition } from "../domain"; import { createDueRunService } from "./dueRunService"; +import { createEventRunService, type EventRunService } from "./eventRunService"; import { processRecordType } from "./processRecordType"; import type { Process, @@ -74,6 +75,11 @@ export interface ProcessService { lockProcesses(ctx: SecurityContext, tx: Tx): Promise; /** Back to a fresh workspace: deletes every run with its tasks, turns every process off, forgets their history. */ resetProcesses(ctx: SecurityContext, tx: Tx): Promise; + /** + * Trusted, as the system, before a transaction commits: starts the runs its events trigger, so + * they commit with the change or not at all. + */ + startRunsOnEvents: EventRunService["startRunsOnEvents"]; } export function createProcessService({ @@ -94,6 +100,7 @@ export function createProcessService({ const reader = createRunReader({ transact, tasks, starter }); const lifecycle = createRunLifecycleService({ orm, transact, logger, tasks, starter, reader }); const dueRuns = createDueRunService({ orm, transact, logger, starter, lifecycle }); + const eventRuns = createEventRunService({ logger, tasks, starter, today }); const historyReader = createRecordHistoryReaderService(orm, { process: processRecordType(orm) }); return { @@ -132,10 +139,9 @@ export function createProcessService({ const subjectId = parseUuid(employeeId); if (!subjectId) throw employeeNotFound(); await ctx.requireInScope("process_run", "create", subjectId); - if (!definition.startableByAdmin) { + if (!definition.startableByAdmin || definition.subject.recordType !== "employee") { throw new ApplicationError("FORBIDDEN", "This process starts only by itself."); } - const workspaceOwnerEmployeeId = await starter.readWorkspaceOwner(); const run = await transact(async (tx) => { await processRepository(tx).lockProcess(process.id); const subject = await starter.readSubject(tx, subjectId); @@ -147,17 +153,21 @@ export function createProcessService({ ); } const startDay = laterOf(today(), subject.startDate); - return starter.start(tx, { - process, - definition, - subject, - startCause: "started_by_admin", - startedByEmployeeId: ctx.user?.employeeId ?? null, - triggerDate: null, - startKey: null, - startDay, - workspaceOwnerEmployeeId, - }); + const [started] = await starter.startMany(tx, [ + { + process, + definition, + record: { recordType: "employee", recordId: subjectId }, + subject, + startCause: "started_by_admin", + startedByEmployeeId: ctx.user?.employeeId ?? null, + triggerDate: null, + triggerEvent: null, + startKey: null, + startDay, + }, + ]); + return started!; }); logger.info({ runId: run.id, processKey, startCause: run.startCause }, "Process run started"); return reader.readDetails(run); @@ -191,6 +201,8 @@ export function createProcessService({ await processRepository(tx).lockAllProcesses(); }, + startRunsOnEvents: (ctx, tx, events) => eventRuns.startRunsOnEvents(ctx, tx, events), + async resetProcesses(ctx, tx) { ctx.requireSetupAccess("MANAGE_WORKSPACE"); await processRepository(tx).reset(); diff --git a/packages/api/src/modules/processes/app/runReader.ts b/packages/api/src/modules/processes/app/runReader.ts index f91c4a3d1..ba4dbf51f 100644 --- a/packages/api/src/modules/processes/app/runReader.ts +++ b/packages/api/src/modules/processes/app/runReader.ts @@ -74,6 +74,7 @@ function runView(run: StoredRun, process: StoredProcess): ProcessRun { subjectEmployeeId: run.subjectEmployeeId, startCause: run.startCause, startedByEmployeeId: run.startedByEmployeeId, + triggerEvent: run.triggerEvent, startDay: run.startDay, status: run.status, finishedAt: run.finishedAt, diff --git a/packages/api/src/modules/processes/app/runStarter.ts b/packages/api/src/modules/processes/app/runStarter.ts index 37680292e..3931266b3 100644 --- a/packages/api/src/modules/processes/app/runStarter.ts +++ b/packages/api/src/modules/processes/app/runStarter.ts @@ -1,6 +1,5 @@ -import { ApplicationError } from "../../../core/applicationError"; import type { Orm, Transact, Tx } from "../../../core/db"; -import { PrincipalSecurityContext, type SecurityContext } from "../../../core/security"; +import { PrincipalSecurityContext } from "../../../core/security"; import { addDays, type CalendarDate } from "../../../utils"; import type { BankHolidayService } from "../../bankHolidays"; import { @@ -8,26 +7,25 @@ import { createEmployeeWorkingPatternService, employeeDirectory, type EmployeeReadService, - type EmployeeWorkingPatternService, } from "../../employees"; -import type { TaskService } from "../../tasks"; -import { processRepository, type StoredProcess } from "../db"; +import type { NewTask, TaskService } from "../../tasks"; +import { type NewRunRow, processRepository, type StoredProcess } from "../db"; import { addDayOffset, type DayOffset, dueTriggerDate, - MAX_SEARCH_DAYS, type IsWorkingDay, parseProcessDefinition, type ProcessDefinition, type ProcessRun, + type RecordDateTrigger, resolveAssignee, + type ResolvedAssignee, type RunStep, - settleRun, type StartCause, - type TaskStep, - workingDaysOf, + type TriggerEvent, } from "../domain"; +import { createWorkingCalendars } from "./workingCalendars"; /** The person a run is about, as starting it needs them. */ export interface RunSubject { @@ -41,30 +39,34 @@ export interface RunSubject { export interface RunStart { process: StoredProcess; definition: ProcessDefinition; - subject: RunSubject; + /** The record the run is about. */ + record: { recordType: ProcessDefinition["subject"]["recordType"]; recordId: string }; + /** The person the run is about: the employee, or a task's subject; null for nobody. */ + subject: RunSubject | null; startCause: StartCause; startedByEmployeeId: string | null; triggerDate: CalendarDate | null; + triggerEvent: TriggerEvent | null; startKey: string | null; startDay: CalendarDate; - workspaceOwnerEmployeeId: string | null; } /** Starts runs: their steps' assignees, tasks and due dates. */ export interface RunStarter { /** The person with this id, or null when there is none. */ readSubject(tx: Tx, employeeId: string): Promise; - /** The owner's employee, read before any lock is taken: it needs its own connection. */ - readWorkspaceOwner(): Promise; + /** The people among these ids that exist, by id. */ + readSubjects(tx: Tx, employeeIds: readonly string[]): Promise>; /** - * Trusted: creates the run, its steps and their tasks in `tx`, as Broccoli, with the run as their - * cause; a run whose steps are all skipped finishes at once. + * Trusted: creates the runs, their steps and their tasks in `tx`, as Broccoli, with each run as + * its tasks' cause, in a fixed number of statements; a run whose steps are all skipped is + * finished at once. Returns the runs in the order of `starts`. */ - start(tx: Tx, start: RunStart): Promise; + startMany(tx: Tx, starts: readonly RunStart[]): Promise; /** The people whose start date triggers a run today, with the date it triggered on. */ findDue( tx: Tx, - definition: ProcessDefinition, + trigger: RecordDateTrigger, days: { today: CalendarDate; turnedOnDay: CalendarDate }, ): Promise<{ employeeId: string; triggerDate: CalendarDate }[]>; /** Each step's title as its task gets it, in the order the run's version lists the steps. */ @@ -83,50 +85,45 @@ export function createRunStarter({ bankHolidays: BankHolidayService; }): RunStarter { const employees = createEmployeeReadService({ orm }); - const calendars: WorkingCalendars = { + const calendars = createWorkingCalendars({ workingPatterns: createEmployeeWorkingPatternService({ transact }), bankHolidays, - }; - const directory = employeeDirectory(orm); - const system = PrincipalSecurityContext.system(); + }); + const readSubjects = (tx: Tx, ids: readonly string[]) => readSubjectsOf(employees, tx, ids); return { - readSubject: (tx, employeeId) => readSubject(employees, tx, employeeId), + readSubject: async (tx, employeeId) => + (await readSubjects(tx, [employeeId])).get(employeeId) ?? null, - readWorkspaceOwner: () => directory.ownersEmployeeId(), + readSubjects, - async start(tx, start) { - const { definition, subject, startDay } = start; - const runs = processRepository(tx); - const run = await runs.insertRun(newRun(start)); - const managers = await employees.readManagerIdsOn(tx, { - employeeIds: [subject.id], - date: startDay, + async startMany(tx, starts) { + if (starts.length === 0) return []; + const assignees = await resolveAssignees(employees, tx, starts); + const workingDays = await calendars.readWorkingDays(tx, { + employeeIds: subjectsCountingBusinessDays(starts), + days: starts.map(({ startDay }) => startDay), }); - const candidates = { - subject: subject.id, - subjects_manager: managers.get(subject.id) ?? null, - workspace_owner: start.workspaceOwnerEmployeeId, - }; - const needsWorkingDays = definition.steps.some(({ due }) => due.unit === "business_days"); - const isWorkingDay = needsWorkingDays - ? await workingDaysAround(calendars, system, tx, subject.id, startDay) - : null; - const steps: RunStep[] = []; - for (const step of definition.steps) { - const assignee = resolveAssignee(step.assignee, candidates); - steps.push( - assignee - ? await startStep(tasks, system, tx, { run, start, step, assignee, isWorkingDay }) - : skippedStep(run.id, step.key), - ); - } - await runs.insertSteps(steps); - const settled = settleRun(run, steps, new Date()); - return settled === run ? run : runs.saveRun(settled); + const now = new Date(); + const runs = await processRepository(tx).insertRuns( + starts.map((start, index) => newRun(start, assignees[index]!, now)), + ); + const plans = runs.map((run, index) => ({ + run, + start: starts[index]!, + assignees: assignees[index]!, + isWorkingDay: workingDays.get(starts[index]!.subject?.id ?? "") ?? null, + })); + const created = await tasks.createTasks( + PrincipalSecurityContext.system(), + tx, + plans.flatMap(newTasksOf), + ); + await processRepository(tx).insertSteps(stepsOf(plans, created, now)); + return runs; }, - async findDue(tx, { trigger }, days) { + async findDue(tx, trigger, days) { // A start date's trigger date is at most the offset's span away from it. const span = spanOf(trigger.offset) + trigger.maxDaysLate; const starts = await employees.readEmployeesStartingBetween(tx, { @@ -143,139 +140,145 @@ export function createRunStarter({ const version = (await processRepository(tx).findVersion(run.processVersionId))!; const { steps } = parseProcessDefinition(version.schemaVersion, version.definition); const subject = run.subjectEmployeeId - ? await readSubject(employees, tx, run.subjectEmployeeId) + ? ((await readSubjects(tx, [run.subjectEmployeeId])).get(run.subjectEmployeeId) ?? null) : null; return new Map(steps.map(({ key, title }) => [key, filled(title, subject)])); }, }; } -/** The person with this id, with their first employment's start, or null when there is none. */ -async function readSubject( +/** One run being started, with what its steps need. */ +interface RunPlan { + run: ProcessRun; + start: RunStart; + /** Each step's assignee, in the definition's order; null for a step skipped for want of one. */ + assignees: (ResolvedAssignee | null)[]; + isWorkingDay: IsWorkingDay | null; +} + +/** The people among these ids that exist, with their first employment's start, by id. */ +async function readSubjectsOf( employees: EmployeeReadService, tx: Tx, - employeeId: string, -): Promise { - const [employee] = await employees.readEmployeesByName(tx, [employeeId]); - if (!employee) return null; - const periods = (await employees.readEmploymentPeriods(tx, [employeeId])).get(employeeId); - return { - id: employee.id, - firstName: employee.firstName, - startDate: periods?.[0]?.startDate ?? null, - }; + employeeIds: readonly string[], +): Promise> { + if (employeeIds.length === 0) return new Map(); + const found = await employees.readEmployeesByName(tx, employeeIds); + const periods = await employees.readEmploymentPeriods( + tx, + found.map(({ id }) => id), + ); + return new Map( + found.map(({ id, firstName }) => [ + id, + { id, firstName, startDate: periods.get(id)?.[0]?.startDate ?? null }, + ]), + ); } -interface WorkingCalendars { - workingPatterns: EmployeeWorkingPatternService; - bankHolidays: BankHolidayService; +/** Each start's step assignees, with each subject's manager on its start day and the owner. */ +async function resolveAssignees( + employees: EmployeeReadService, + tx: Tx, + starts: readonly RunStart[], +): Promise<(ResolvedAssignee | null)[][]> { + const owner = await employeeDirectory(tx).ownersEmployeeId(); + const managersByDay = new Map>(); + for (const day of new Set(starts.map(({ startDay }) => startDay))) { + const employeeIds = starts.flatMap((start) => + start.startDay === day && start.subject ? [start.subject.id] : [], + ); + managersByDay.set(day, await employees.readManagerIdsOn(tx, { employeeIds, date: day })); + } + return starts.map(({ definition, subject, startDay }) => { + const candidates = { + subject: subject?.id ?? null, + subjects_manager: subject ? (managersByDay.get(startDay)!.get(subject.id) ?? null) : null, + workspace_owner: owner, + }; + return definition.steps.map((step) => resolveAssignee(step.assignee, candidates)); + }); +} + +/** The subjects of runs with a step due in business days, which count their own working days. */ +function subjectsCountingBusinessDays(starts: readonly RunStart[]): string[] { + const ids = starts.flatMap(({ definition, subject }) => + subject && definition.steps.some(({ due }) => due.unit === "business_days") ? [subject.id] : [], + ); + return [...new Set(ids)]; } -function newRun({ - process, - subject, - startCause, - startedByEmployeeId, - triggerDate, - startKey, - startDay, -}: RunStart) { +function newRun(start: RunStart, assignees: readonly (ResolvedAssignee | null)[], now: Date) { + const finished = assignees.every((assignee) => assignee === null); return { - processId: process.id, - processVersionId: process.activeVersion.id, - recordType: "employee", - recordId: subject.id, - subjectEmployeeId: subject.id, - startCause, - startedByEmployeeId, - triggerDate, - startKey, - startDay, - status: "in_progress" as const, - finishedAt: null, + processId: start.process.id, + processVersionId: start.process.activeVersion.id, + recordType: start.record.recordType, + recordId: start.record.recordId, + subjectEmployeeId: start.subject?.id ?? null, + startCause: start.startCause, + startedByEmployeeId: start.startedByEmployeeId, + triggerDate: start.triggerDate, + triggerEvent: start.triggerEvent, + startKey: start.startKey, + startDay: start.startDay, + status: finished ? "finished" : "in_progress", + finishedAt: finished ? now : null, cancelledAt: null, cancelledByEmployeeId: null, cancelCause: null, cancelReason: null, - }; + } satisfies NewRunRow; } -/** Gives the step's task to its assignee, due its offset after the start day. */ -async function startStep( - tasks: TaskService, - ctx: SecurityContext, - tx: Tx, - { - run, - start: { process, subject, startDay }, - step, - assignee, - isWorkingDay, - }: { - run: ProcessRun; - start: RunStart; - step: TaskStep; - assignee: { employeeId: string; note: RunStep["assigneeNote"] }; - isWorkingDay: IsWorkingDay | null; - }, -): Promise { - const task = await tasks.createTask(ctx, tx, { - type: "process_step_to_do", - title: filled(step.title, subject), - description: step.description === null ? null : filled(step.description, subject), - assigneeEmployeeId: assignee.employeeId, - processSource: { processKey: process.key, processRunId: run.id, stepKey: step.key }, - subjectEmployeeId: subject.id, - record: { recordType: run.recordType, recordId: run.recordId }, - dueDate: addDayOffset(startDay, step.due, isWorkingDay), - cause: { kind: "PROCESS_RUN", ref: run.id }, +/** The tasks of a run's assigned steps, due their offset after its start day. */ +function newTasksOf({ run, start, assignees, isWorkingDay }: RunPlan): NewTask[] { + return start.definition.steps.flatMap((step, index) => { + const assignee = assignees[index]; + if (!assignee) return []; + return [ + { + type: "process_step_to_do", + title: filled(step.title, start.subject), + description: step.description === null ? null : filled(step.description, start.subject), + assigneeEmployeeId: assignee.employeeId, + processSource: { processKey: start.process.key, processRunId: run.id, stepKey: step.key }, + subjectEmployeeId: start.subject?.id ?? null, + record: { recordType: run.recordType, recordId: run.recordId }, + dueDate: addDayOffset(start.startDay, step.due, isWorkingDay), + cause: { kind: "PROCESS_RUN", ref: run.id }, + }, + ]; }); - return { - runId: run.id, - stepKey: step.key, - status: "step_awaiting_task_completion", - taskId: task.id, - assigneeNote: assignee.note, - skipReason: null, - finishedAt: null, - }; } -/** Business days by the person's own working weeks and bank holidays, refusing a year with none known. */ -async function workingDaysAround( - { workingPatterns, bankHolidays }: WorkingCalendars, - ctx: SecurityContext, - tx: Tx, - employeeId: string, - day: CalendarDate, -): Promise { - const from = addDays(day, -MAX_SEARCH_DAYS); - const until = addDays(day, MAX_SEARCH_DAYS); - const weeks = - ( - await workingPatterns.readWorkingWeeksInTransaction(tx, { - employeeIds: [employeeId], - from, - until, - }) - ).get(employeeId) ?? []; - const calendarNames = [...new Set(weeks.flatMap(({ bankHolidayCalendar: c }) => (c ? [c] : [])))]; - const holidays = await bankHolidays.readBankHolidays(ctx, { - calendars: calendarNames, - from, - until, - }); - return workingDaysOf(weeks, (date, weekIndex) => { - const calendar = weeks[weekIndex]?.bankHolidayCalendar; - if (!calendar) return false; - const days = holidays.get(calendar) ?? []; - const year = date.slice(0, 4); - // Every UK calendar has bank holidays each year: none means the data ends before it. - if (!days.some((holiday) => holiday.date.startsWith(year))) { - throw new ApplicationError("CONFLICT", `No bank holidays are known for ${year}.`); - } - return days.some((holiday) => holiday.date === date); - }); +/** Every run's steps: awaiting the task created for it, in the same order, or skipped. */ +function stepsOf(plans: readonly RunPlan[], created: readonly { id: string }[], now: Date) { + let next = 0; + return plans.flatMap(({ run, start, assignees }) => + start.definition.steps.map((step, index): RunStep => { + const assignee = assignees[index]; + return assignee + ? { + runId: run.id, + stepKey: step.key, + status: "step_awaiting_task_completion", + taskId: created[next++]!.id, + assigneeNote: assignee.note, + skipReason: null, + finishedAt: null, + } + : { + runId: run.id, + stepKey: step.key, + status: "step_skipped", + taskId: null, + assigneeNote: null, + skipReason: "no_assignee_for_step", + finishedAt: now, + }; + }), + ); } /** How far a trigger date can be from its record's date: business days stretch by weekends and holidays. */ @@ -286,15 +289,3 @@ function spanOf({ amount, unit }: DayOffset): number { function filled(text: string, subject: Pick | null): string { return text.replaceAll("{firstName}", subject?.firstName ?? "them"); } - -function skippedStep(runId: string, stepKey: string): RunStep { - return { - runId, - stepKey, - status: "step_skipped", - taskId: null, - assigneeNote: null, - skipReason: "no_assignee_for_step", - finishedAt: new Date(), - }; -} diff --git a/packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts b/packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts new file mode 100644 index 000000000..9b825e95c --- /dev/null +++ b/packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts @@ -0,0 +1,340 @@ +import { assert, onTestFinished, vi } from "vitest"; + +import { + ADMIN, + calendarDate, + employeeServices, + expect, + processService, + signedIn, + taskService, + test, + transactWithProcesses, +} from "#test/support"; + +import type { ProcessDefinition } from "../domain"; + +test("adding a person commits exactly one process run and a task for their manager", async ({ + orm, + seed, +}) => { + vi.useFakeTimers({ toFake: ["Date"], now: new Date("2026-12-03T12:00:00Z") }); + onTestFinished(() => { + vi.useRealTimers(); + }); + const manager = await seed.employee(); + const definition = eventProcess("employees.added"); + definition.steps[0]!.assignee = { kind: "subjects_manager" }; + const { key } = await seed.process({ key: "welcome", definition }); + const transact = transactWithProcesses(orm, { today }); + + const employee = await transact(async (tx) => { + const employees = employeeServices(orm, { transact: (work) => work(tx) }); + const added = await employees.createEmployee(ADMIN, { + firstName: "Sam", + lastName: "New", + email: "sam@example.com", + }); + assert(added); + await employees.setManagers(ADMIN, { employeeIds: [added.id], managerId: manager.id }); + return added; + }); + + expect(await employeeServices(orm).readEmployee(ADMIN, employee.id)).toEqual(employee); + expect(await processService(orm).listRuns(ADMIN, key)).toMatchObject([ + { + startCause: "started_on_event", + startDay: "2026-12-03", + subjectEmployeeId: employee.id, + triggerEvent: { + type: "employees.added", + payload: { employeeIds: [employee.id] }, + }, + record: { recordType: "employee", recordId: employee.id }, + }, + ]); + const ctx = signedIn(orm, { employeeId: manager.id, profiles: ["EMPLOYEE"] }); + expect((await taskService(orm).listMyTasks(ctx)).needsYou).toMatchObject([ + { + subjectEmployeeId: employee.id, + processSource: { processKey: key }, + }, + ]); +}); + +test("a failed transaction leaves no event-started run", async ({ orm, seed }) => { + const employee = await seed.employee(); + const { key } = await seed.process({ + key: "welcome", + definition: eventProcess("employees.added"), + }); + const transact = transactWithProcesses(orm, { today }); + + await expect( + transact((tx) => { + tx.publish("employees.added", { employeeIds: [employee.id] }); + throw new Error("Save failed"); + }), + ).rejects.toThrow("Save failed"); + + expect(await processService(orm).listRuns(ADMIN, key)).toEqual([]); + const ctx = signedIn(orm, { employeeId: employee.id, profiles: ["EMPLOYEE"] }); + expect((await taskService(orm).listMyTasks(ctx)).needsYou).toEqual([]); +}); + +test("one event for three people starts three runs", async ({ orm, seed }) => { + const people = await seed.employees(3); + const ids = people.map(({ id }) => id); + const { key } = await seed.process({ + key: "welcome", + definition: eventProcess("employees.added"), + }); + const transact = transactWithProcesses(orm, { today }); + + await transact((tx) => { + tx.publish("employees.added", { employeeIds: ids }); + return Promise.resolve(); + }); + + const runs = await processService(orm).listRuns(ADMIN, key); + expect(runs.map(({ record }) => record.recordId).sort()).toEqual([...ids].sort()); + expect(new Set(runs.map(({ triggerEvent }) => triggerEvent!.id)).size).toBe(1); +}); + +test("three events in one transaction start three runs", async ({ orm, seed }) => { + const people = await seed.employees(3); + const ids = people.map(({ id }) => id); + const { key } = await seed.process({ + key: "welcome", + definition: eventProcess("employees.added"), + }); + const transact = transactWithProcesses(orm, { today }); + + await transact((tx) => { + for (const id of ids) tx.publish("employees.added", { employeeIds: [id] }); + return Promise.resolve(); + }); + + const runs = await processService(orm).listRuns(ADMIN, key); + expect(runs.map(({ record }) => record.recordId).sort()).toEqual([...ids].sort()); + expect(new Set(runs.map(({ triggerEvent }) => triggerEvent!.id)).size).toBe(3); +}); + +test("a run that cannot calculate its due date rolls back adding the person", async ({ + orm, + seed, +}) => { + const definition = eventProcess("employees.added"); + definition.steps[0]!.due = { amount: 1, unit: "business_days" }; + const { key } = await seed.process({ key: "welcome", definition }); + const transact = transactWithProcesses(orm, { today: () => calendarDate("2031-06-02") }); + const employees = employeeServices(orm, { transact }); + + await expect( + employees.createEmployee(ADMIN, { + firstName: "Sam", + lastName: "New", + email: "sam@example.com", + }), + ).rejects.toMatchObject({ code: "CONFLICT", message: "No bank holidays are known for 2031." }); + + expect((await employees.listEmployees(ADMIN, {})).edges).toEqual([]); + expect(await processService(orm).listRuns(ADMIN, key)).toEqual([]); +}); + +test("an off process starts nothing when its event happens", async ({ orm, seed }) => { + const { key } = await seed.process({ + key: "welcome", + definition: eventProcess("employees.added"), + turnedOn: false, + }); + const transact = transactWithProcesses(orm, { today }); + + await employeeServices(orm, { transact }).createEmployee(ADMIN, { + firstName: "Sam", + lastName: "New", + email: "sam@example.com", + }); + + expect(await processService(orm).listRuns(ADMIN, key)).toEqual([]); +}); + +test("an unmatched event condition starts nothing", async ({ orm, seed }) => { + const definition = eventProcess("employees.added", { field: "source", in: ["import"] }); + const { key } = await seed.process({ key: "welcome", definition }); + const transact = transactWithProcesses(orm, { today }); + + await employeeServices(orm, { transact }).createEmployee(ADMIN, { + firstName: "Sam", + lastName: "New", + email: "sam@example.com", + }); + + expect(await processService(orm).listRuns(ADMIN, key)).toEqual([]); +}); + +test("marking a task done starts a process carrying the person who acted", async ({ + orm, + seed, +}) => { + const assignee = await seed.employee(); + const subject = await seed.employee(); + const task = await seed.task({ + assigneeEmployeeId: assignee.id, + askedByEmployeeId: assignee.id, + subjectEmployeeId: subject.id, + }); + const definition = eventProcess("tasks.task_status_changed", { + field: "to", + in: ["done", "cancelled"], + }); + const { key } = await seed.process({ key: "follow_up", definition }); + const transact = transactWithProcesses(orm, { today }); + const ctx = signedIn(orm, { employeeId: assignee.id, profiles: ["EMPLOYEE"] }); + + await taskService(orm, { transact }).markDone(ctx, task.id); + + expect(await processService(orm).listRuns(ADMIN, key)).toMatchObject([ + { + startCause: "started_on_event", + subjectEmployeeId: subject.id, + record: { recordType: "task", recordId: task.id }, + triggerEvent: { + type: "tasks.task_status_changed", + payload: { + taskId: task.id, + from: "to_do", + to: "done", + byEmployeeId: assignee.id, + }, + }, + }, + ]); +}); + +test("marking a task waiting does not start a done-or-cancelled process", async ({ orm, seed }) => { + const assignee = await seed.employee(); + const task = await seed.task({ assigneeEmployeeId: assignee.id, askedByEmployeeId: assignee.id }); + const definition = eventProcess("tasks.task_status_changed", { + field: "to", + in: ["done", "cancelled"], + }); + const { key } = await seed.process({ key: "follow_up", definition }); + const transact = transactWithProcesses(orm, { today }); + const tasks = taskService(orm, { transact }); + const ctx = signedIn(orm, { employeeId: assignee.id, profiles: ["EMPLOYEE"] }); + + await transact((tx) => tasks.markWaiting(ctx, tx, task.id)); + + expect((await tasks.readTask(ctx, task.id)).status).toBe("waiting"); + expect(await processService(orm).listRuns(ADMIN, key)).toEqual([]); +}); + +test("a run's tasks start another process in the same transaction", async ({ orm, seed }) => { + const first = await seed.process({ key: "welcome", definition: eventProcess("employees.added") }); + const definition = eventProcess("tasks.task_created", { field: "processKey", in: [first.key] }); + const second = await seed.process({ key: "follow_up", definition }); + const transact = transactWithProcesses(orm, { today }); + + const added = await employeeServices(orm, { transact }).createEmployee(ADMIN, { + firstName: "Sam", + lastName: "New", + email: "sam@example.com", + }); + + assert(added); + const processes = processService(orm); + const [run] = await processes.listRuns(ADMIN, first.key); + const details = await processes.readRun(ADMIN, run!.id); + const task = details.steps[0]!.task!; + expect(await processes.listRuns(ADMIN, second.key)).toMatchObject([ + { + subjectEmployeeId: added.id, + record: { recordType: "task", recordId: task.id }, + triggerEvent: { + type: "tasks.task_created", + payload: { taskId: task.id, processKey: first.key }, + }, + }, + ]); + const ctx = signedIn(orm, { employeeId: added.id, profiles: ["EMPLOYEE"] }); + expect((await taskService(orm).listMyTasks(ctx)).needsYou).toHaveLength(2); +}); + +test("two matching events start two runs when every step is skipped", async ({ orm, seed }) => { + const employee = await seed.employee(); + const definition = eventProcess("employees.added"); + definition.steps[0]!.assignee = { kind: "subjects_manager" }; + const { key } = await seed.process({ key: "welcome", definition }); + const transact = transactWithProcesses(orm, { today }); + + await transact((tx) => { + tx.publish("employees.added", { employeeIds: [employee.id] }); + tx.publish("employees.added", { employeeIds: [employee.id] }); + return Promise.resolve(); + }); + + const runs = await processService(orm).listRuns(ADMIN, key); + expect(runs).toMatchObject([{ status: "finished" }, { status: "finished" }]); + expect(runs[0]!.triggerEvent!.id).not.toBe(runs[1]!.triggerEvent!.id); +}); + +test("a matching event during a run in progress is skipped and the save commits", async ({ + orm, + seed, +}) => { + const employee = await seed.employee(); + const { key } = await seed.process({ + key: "welcome", + definition: eventProcess("employees.added"), + }); + const transact = transactWithProcesses(orm, { today }); + await transact((tx) => { + tx.publish("employees.added", { employeeIds: [employee.id] }); + return Promise.resolve(); + }); + const before = await processService(orm).listRuns(ADMIN, key); + + await transact(async (tx) => { + await employeeServices(orm, { transact: (work) => work(tx) }).updateEmployee(ADMIN, { + id: employee.id, + jobTitle: "Engineer", + }); + tx.publish("employees.added", { employeeIds: [employee.id] }); + }); + + expect(await processService(orm).listRuns(ADMIN, key)).toEqual(before); + expect(before).toMatchObject([{ status: "in_progress" }]); + expect(await employeeServices(orm).readEmployee(ADMIN, employee.id)).toMatchObject({ + jobTitle: "Engineer", + }); +}); + +const today = () => calendarDate("2026-12-03"); + +function eventProcess( + event: "employees.added" | "tasks.task_created" | "tasks.task_status_changed", + when?: { field: string; in: (string | null)[] }, +): ProcessDefinition { + return { + subject: { recordType: event === "employees.added" ? "employee" : "task" }, + trigger: { + kind: "on_event", + event, + run: "in_same_transaction", + faultMode: "rollback_transaction", + ...(when && { when }), + }, + startableByAdmin: false, + steps: [ + { + key: "follow_up", + kind: "task", + title: "Follow up", + description: null, + assignee: { kind: "subject" }, + due: { amount: 0, unit: "calendar_days" }, + }, + ], + }; +} diff --git a/packages/api/src/modules/processes/app/workingCalendars.ts b/packages/api/src/modules/processes/app/workingCalendars.ts new file mode 100644 index 000000000..9b48e3815 --- /dev/null +++ b/packages/api/src/modules/processes/app/workingCalendars.ts @@ -0,0 +1,76 @@ +import { ApplicationError } from "../../../core/applicationError"; +import type { Tx } from "../../../core/db"; +import { PrincipalSecurityContext } from "../../../core/security"; +import { addDays, type CalendarDate } from "../../../utils"; +import type { BankHolidayService } from "../../bankHolidays"; +import type { EmployeeWorkingPatternService } from "../../employees"; +import { type IsWorkingDay, MAX_SEARCH_DAYS, workingDaysOf } from "../domain"; + +/** People's working days, by their own working weeks and bank holidays, for counting business days. */ +export interface WorkingCalendars { + /** + * Each person's working days around these days, refusing a year with no bank holidays known; + * null for a person on no terms, whose offsets count calendar days. + */ + readWorkingDays( + tx: Tx, + input: { employeeIds: readonly string[]; days: readonly CalendarDate[] }, + ): Promise>; +} + +export function createWorkingCalendars({ + workingPatterns, + bankHolidays, +}: { + workingPatterns: EmployeeWorkingPatternService; + bankHolidays: BankHolidayService; +}): WorkingCalendars { + return { + async readWorkingDays(tx, { employeeIds, days }) { + if (employeeIds.length === 0) return new Map(); + const sorted = [...days].sort(); + const from = addDays(sorted[0]!, -MAX_SEARCH_DAYS); + const until = addDays(sorted.at(-1)!, MAX_SEARCH_DAYS); + const weeksByEmployee = await workingPatterns.readWorkingWeeksInTransaction(tx, { + employeeIds, + from, + until, + }); + const calendarNames = [ + ...new Set( + [...weeksByEmployee.values()].flatMap((weeks) => + weeks.flatMap(({ bankHolidayCalendar: c }) => (c ? [c] : [])), + ), + ), + ]; + const holidays = await bankHolidays.readBankHolidays(PrincipalSecurityContext.system(), { + calendars: calendarNames, + from, + until, + }); + return new Map( + employeeIds.map((employeeId) => { + const weeks = weeksByEmployee.get(employeeId) ?? []; + return [employeeId, workingDaysOf(weeks, isBankHolidayIn(weeks, holidays))]; + }), + ); + }, + }; +} + +function isBankHolidayIn( + weeks: readonly { bankHolidayCalendar: string | null }[], + holidays: ReadonlyMap, +) { + return (date: CalendarDate, weekIndex: number): boolean => { + const calendar = weeks[weekIndex]?.bankHolidayCalendar; + if (!calendar) return false; + const days = holidays.get(calendar) ?? []; + const year = date.slice(0, 4); + // Every UK calendar has bank holidays each year: none means the data ends before it. + if (!days.some((holiday) => holiday.date.startsWith(year))) { + throw new ApplicationError("CONFLICT", `No bank holidays are known for ${year}.`); + } + return days.some((holiday) => holiday.date === date); + }; +} diff --git a/packages/api/src/modules/processes/db/index.ts b/packages/api/src/modules/processes/db/index.ts index 1a43b56ec..51b2fa362 100644 --- a/packages/api/src/modules/processes/db/index.ts +++ b/packages/api/src/modules/processes/db/index.ts @@ -1 +1,6 @@ -export { processRepository, type StoredProcess } from "./processRepository"; +export { + type NewProcessRow, + type NewRunRow, + processRepository, + type StoredProcess, +} from "./processRepository"; diff --git a/packages/api/src/modules/processes/db/processRepository.ts b/packages/api/src/modules/processes/db/processRepository.ts index 38ce16aff..87ac9200d 100644 --- a/packages/api/src/modules/processes/db/processRepository.ts +++ b/packages/api/src/modules/processes/db/processRepository.ts @@ -1,8 +1,8 @@ -import { and, desc, eq, inArray, sql } from "drizzle-orm"; +import { and, desc, eq, inArray, isNotNull, type SQL, sql } from "drizzle-orm"; -import { ormOf, type Orm, type Tx } from "../../../core/db"; +import { ormOf, type Orm, type OrmTransaction, type Tx } from "../../../core/db"; import { inRecordScope, type RecordScope } from "../../../core/security"; -import type { ProcessRun, RunStep } from "../domain"; +import { PROCESS_DEFINITION_SCHEMA_VERSION, type ProcessRun, type RunStep } from "../domain"; import { processes, processVersions } from "./processesTable"; import { processRuns, processRunSteps } from "./processRunsTable"; @@ -16,6 +16,15 @@ export interface StoredProcess { activeVersion: { id: string; version: number; schemaVersion: number; definition: unknown }; } +/** A process to insert, with the definition of its first version. */ +export interface NewProcessRow { + key: string; + name: string; + description: string; + definition: unknown; + turnedOnAt: Date | null; +} + /** A run as it is stored before it has an id and timestamps. */ export type NewRunRow = Omit; @@ -30,8 +39,17 @@ export interface ProcessRepository { lockProcess(id: string): Promise; /** Every process, locked until the transaction ends, in id order. */ lockAllProcesses(): Promise; + /** + * The turned-on processes whose active version starts on one of these events, share-locked until + * the transaction ends: starts from concurrent changes never wait for each other, while turning + * a process off or a reset waits for them. + */ + lockProcessesStartedBy(eventTypes: readonly string[]): Promise; + /** A new process with its first version, published and active; for tests and seeds. */ + insertProcess(process: NewProcessRow): Promise<{ id: string; key: string }>; setTurnedOnAt(id: string, turnedOnAt: Date | null): Promise; - insertRun(run: NewRunRow): Promise; + /** Inserts the runs in one statement; returns them in the same order. */ + insertRuns(runs: readonly NewRunRow[]): Promise; insertSteps(steps: readonly RunStep[]): Promise; findRun(id: string): Promise; /** The run, locked until the transaction ends, so step changes on it take turns. */ @@ -50,6 +68,12 @@ export interface ProcessRepository { recordType: string, recordIds: readonly string[], ): Promise>; + /** The records among these with a run of the process in progress. */ + findRecordsWithRunInProgress( + processId: string, + recordType: string, + recordIds: readonly string[], + ): Promise>; /** The record's run of the process in progress, if any. */ findRunInProgress( processId: string, @@ -112,6 +136,39 @@ export function processRepository(db: Orm | Tx): ProcessRepository { await orm.select({ id: processes.id }).from(processes).orderBy(processes.id).for("update"); }, + async lockProcessesStartedBy(eventTypes) { + const rows = await selectProcesses() + .where( + and( + isNotNull(processes.turnedOnAt), + sql`${processVersions.definition} -> 'trigger' ->> 'kind' = 'on_event'`, + sql`${processVersions.definition} -> 'trigger' ->> 'event' = any(${sql.param([...eventTypes])}::text[])`, + ), + ) + .orderBy(processes.id) + .for("share", { of: processes }); + return rows.map(toStoredProcess); + }, + + async insertProcess({ definition, ...process }) { + const [inserted] = await orm.insert(processes).values(process).returning(); + const [version] = await orm + .insert(processVersions) + .values({ + processId: inserted!.id, + version: 1, + status: "published", + definitionSchemaVersion: PROCESS_DEFINITION_SCHEMA_VERSION, + definition, + }) + .returning(); + await orm + .update(processes) + .set({ activeVersionId: version!.id }) + .where(eq(processes.id, inserted!.id)); + return { id: inserted!.id, key: inserted!.key }; + }, + async setTurnedOnAt(id, turnedOnAt) { await orm .update(processes) @@ -119,9 +176,12 @@ export function processRepository(db: Orm | Tx): ProcessRepository { .where(eq(processes.id, id)); }, - async insertRun(run) { - const [row] = await orm.insert(processRuns).values(run).returning(); - return row!; + async insertRuns(runs) { + if (runs.length === 0) return []; + return await orm + .insert(processRuns) + .values([...runs]) + .returning(); }, async insertSteps(steps) { @@ -181,20 +241,15 @@ export function processRepository(db: Orm | Tx): ProcessRepository { return await orm.select().from(processRuns).where(eq(processRuns.status, "in_progress")); }, - async findRecordsWithRuns(processId, recordType, recordIds) { - if (recordIds.length === 0) return new Set(); - const rows = await orm - .selectDistinct({ recordId: processRuns.recordId }) - .from(processRuns) - .where( - and( - eq(processRuns.processId, processId), - eq(processRuns.recordType, recordType), - sql`${processRuns.recordId} = any(${sql.param([...recordIds])}::text[])`, - ), - ); - return new Set(rows.map(({ recordId }) => recordId)); - }, + findRecordsWithRuns: (processId, recordType, recordIds) => + findRecordsWithRunsWhere(orm, { processId, recordType, recordIds }), + + findRecordsWithRunInProgress: (processId, recordType, recordIds) => + findRecordsWithRunsWhere( + orm, + { processId, recordType, recordIds }, + eq(processRuns.status, "in_progress"), + ), async findRunInProgress(processId, recordType, recordId) { const [row] = await orm @@ -218,6 +273,31 @@ export function processRepository(db: Orm | Tx): ProcessRepository { }; } +/** The records among these with a run of the process that also meets `condition`. */ +async function findRecordsWithRunsWhere( + orm: Orm | OrmTransaction, + { + processId, + recordType, + recordIds, + }: { processId: string; recordType: string; recordIds: readonly string[] }, + condition?: SQL, +): Promise> { + if (recordIds.length === 0) return new Set(); + const rows = await orm + .selectDistinct({ recordId: processRuns.recordId }) + .from(processRuns) + .where( + and( + eq(processRuns.processId, processId), + eq(processRuns.recordType, recordType), + sql`${processRuns.recordId} = any(${sql.param([...recordIds])}::text[])`, + condition, + ), + ); + return new Set(rows.map(({ recordId }) => recordId)); +} + type ProcessWithVersionRow = { process: typeof processes.$inferSelect; version: StoredProcess["activeVersion"]; diff --git a/packages/api/src/modules/processes/db/processRunsTable.ts b/packages/api/src/modules/processes/db/processRunsTable.ts index a6ad8dc68..d39e4a768 100644 --- a/packages/api/src/modules/processes/db/processRunsTable.ts +++ b/packages/api/src/modules/processes/db/processRunsTable.ts @@ -12,6 +12,7 @@ import { uuid, } from "drizzle-orm/pg-core"; +import { jsonb } from "../../../core/db"; import type { CalendarDate } from "../../../utils"; import { ASSIGNEE_NOTES, @@ -20,6 +21,7 @@ import { SKIP_REASONS, START_CAUSES, STEP_STATUSES, + type TriggerEvent, } from "../domain"; import { processes, processVersions } from "./processesTable"; @@ -43,6 +45,7 @@ export const processRuns = pgTable( startedByEmployeeId: uuid("started_by_employee_id"), triggerDate: date("trigger_date", { mode: "string" }).$type(), startKey: text("start_key"), + triggerEvent: jsonb("trigger_event").$type(), startDay: date("start_day", { mode: "string" }).$type().notNull(), status: text("status", { enum: RUN_STATUSES }).notNull(), finishedAt: timestamp("finished_at", { withTimezone: true }), @@ -80,7 +83,11 @@ export const processRuns = pgTable( ), check( "process_runs_start_cause_known", - sql`${table.startCause} in ('started_on_record_date', 'started_by_admin')`, + sql`${table.startCause} in ('started_on_record_date', 'started_by_admin', 'started_on_event')`, + ), + check( + "process_runs_trigger_event_follows_start_cause", + sql`(${table.startCause} = 'started_on_event') = (${table.triggerEvent} is not null)`, ), check( "process_runs_cancel_cause_known", diff --git a/packages/api/src/modules/processes/domain/eventTrigger.test.ts b/packages/api/src/modules/processes/domain/eventTrigger.test.ts new file mode 100644 index 000000000..7a9c57913 --- /dev/null +++ b/packages/api/src/modules/processes/domain/eventTrigger.test.ts @@ -0,0 +1,107 @@ +import { expect, test } from "vitest"; + +import { matchEventStarts } from "./eventTrigger"; +import type { ProcessDefinition } from "./processDefinition"; + +test("starts each added person in event, process and record order", () => { + const processes = [ + { processId: "welcome", trigger: onEvent("employees.added") }, + { processId: "induction", trigger: onEvent("employees.added") }, + ]; + const event = { + id: "event-1", + type: "employees.added", + payload: { employeeIds: ["sam", "alex", "jo"] }, + }; + + expect(matchEventStarts(processes, [event])).toEqual([ + { processId: "welcome", event, recordId: "sam" }, + { processId: "welcome", event, recordId: "alex" }, + { processId: "welcome", event, recordId: "jo" }, + { processId: "induction", event, recordId: "sam" }, + { processId: "induction", event, recordId: "alex" }, + { processId: "induction", event, recordId: "jo" }, + ]); +}); + +test.each(["tasks.task_created", "tasks.task_status_changed"] as const)( + "starts one run for the task in %s", + (type) => { + const event = { id: "event-1", type, payload: { taskId: "task-1" } }; + + expect(matchEventStarts([{ processId: "follow_up", trigger: onEvent(type) }], [event])).toEqual( + [{ processId: "follow_up", event, recordId: "task-1" }], + ); + }, +); + +test.each([ + { payload: { taskId: "task-1", to: "done" }, accepted: ["done", "cancelled"], matches: true }, + { + payload: { taskId: "task-1", to: "cancelled" }, + accepted: ["done", "cancelled"], + matches: true, + }, + { payload: { taskId: "task-1", to: "waiting" }, accepted: ["done", "cancelled"], matches: false }, + { payload: { taskId: "task-1", to: null }, accepted: ["done"], matches: false }, + { payload: { taskId: "task-1" }, accepted: ["done"], matches: false }, + { payload: { taskId: "task-1", to: null }, accepted: [null], matches: true }, + { payload: { taskId: "task-1" }, accepted: [null], matches: true }, +])("matches $payload against $accepted: $matches", ({ payload, accepted, matches }) => { + const event = { id: "event-1", type: "tasks.task_status_changed", payload }; + const trigger = onEvent("tasks.task_status_changed", { field: "to", in: accepted }); + + expect(matchEventStarts([{ processId: "follow_up", trigger }], [event])).toEqual( + matches ? [{ processId: "follow_up", event, recordId: "task-1" }] : [], + ); +}); + +test("does not start a date-triggered process from an event", () => { + const trigger: ProcessDefinition["trigger"] = { + kind: "on_record_date", + recordType: "employee", + dateField: "startDate", + offset: { amount: 0, unit: "calendar_days" }, + maxDaysLate: 7, + }; + const event = { id: "event-1", type: "employees.added", payload: { employeeIds: ["sam"] } }; + + expect(matchEventStarts([{ processId: "welcome", trigger }], [event])).toEqual([]); +}); + +test("keeps two matching events for the same record as separate starts", () => { + const first = { id: "event-1", type: "employees.added", payload: { employeeIds: ["sam"] } }; + const second = { ...first, id: "event-2" }; + const processes = [ + { processId: "welcome", trigger: onEvent("employees.added") }, + { processId: "induction", trigger: onEvent("employees.added") }, + ]; + + expect(matchEventStarts(processes, [first, second])).toEqual([ + { processId: "welcome", event: first, recordId: "sam" }, + { processId: "induction", event: first, recordId: "sam" }, + { processId: "welcome", event: second, recordId: "sam" }, + { processId: "induction", event: second, recordId: "sam" }, + ]); +}); + +test("does not start a process for an event of another type", () => { + const event = { id: "event-1", type: "tasks.task_created", payload: { taskId: "task-1" } }; + + expect( + matchEventStarts([{ processId: "welcome", trigger: onEvent("employees.added") }], [event]), + ).toEqual([]); +}); + +function onEvent( + event: "employees.added" | "tasks.task_created" | "tasks.task_status_changed", + when?: { field: string; in: (string | null)[] }, +): ProcessDefinition["trigger"] { + return { + kind: "on_event", + event, + run: "in_same_transaction", + faultMode: "rollback_transaction", + ...(when && { when }), + }; +} diff --git a/packages/api/src/modules/processes/domain/eventTrigger.ts b/packages/api/src/modules/processes/domain/eventTrigger.ts new file mode 100644 index 000000000..d8d1e6779 --- /dev/null +++ b/packages/api/src/modules/processes/domain/eventTrigger.ts @@ -0,0 +1,73 @@ +import type { DomainEvents } from "../../../core/events"; +import type { EventTrigger, ProcessDefinition } from "./processDefinition"; + +/** + * The events that may start a process, with the record type each is about and the ids of those + * records in its payload: one run per record. + */ +// shortcut: the list lives here; move it to the publishing modules' registrations when modules become dynamic. +export const TRIGGERING_EVENTS = { + "employees.added": { + recordType: "employee", + recordIds: ({ employeeIds }: DomainEvents["employees.added"]) => employeeIds, + }, + "tasks.task_created": { + recordType: "task", + recordIds: ({ taskId }: DomainEvents["tasks.task_created"]) => [taskId], + }, + "tasks.task_status_changed": { + recordType: "task", + recordIds: ({ taskId }: DomainEvents["tasks.task_status_changed"]) => [taskId], + }, +} as const; + +export type TriggeringEventType = keyof typeof TRIGGERING_EVENTS; + +/** An event as a trigger reads it. */ +export interface TriggerEvent { + id: string; + type: string; + payload: unknown; +} + +/** One run to start: a process, the event that triggered it and one record the event is about. */ +export interface EventStart { + processId: string; + event: Event; + recordId: string; +} + +/** + * The runs the events start, one per matching process and record, in the order of the events, + * then the processes, then the event's records. + */ +export function matchEventStarts( + processes: readonly { processId: string; trigger: ProcessDefinition["trigger"] }[], + events: readonly Event[], +): EventStart[] { + return events.flatMap((event) => + processes.flatMap(({ processId, trigger }) => + trigger.kind === "on_event" && isTriggeredBy(trigger, event) + ? recordIdsOf(event).map((recordId) => ({ processId, event, recordId })) + : [], + ), + ); +} + +export function isTriggeringEvent(type: string): type is TriggeringEventType { + return Object.hasOwn(TRIGGERING_EVENTS, type); +} + +function isTriggeredBy(trigger: EventTrigger, { type, payload }: TriggerEvent): boolean { + if (type !== trigger.event) return false; + if (!trigger.when) return true; + const value = (payload as Record)[trigger.when.field] ?? null; + return trigger.when.in.some((expected) => expected === value); +} + +function recordIdsOf({ type, payload }: TriggerEvent): readonly string[] { + if (!isTriggeringEvent(type)) return []; + // Each entry reads its own event's payload, which `type` names. + const { recordIds } = TRIGGERING_EVENTS[type] as { recordIds: (payload: unknown) => string[] }; + return recordIds(payload); +} diff --git a/packages/api/src/modules/processes/domain/index.ts b/packages/api/src/modules/processes/domain/index.ts index daa3dcf9d..64893cfd1 100644 --- a/packages/api/src/modules/processes/domain/index.ts +++ b/packages/api/src/modules/processes/domain/index.ts @@ -1,9 +1,19 @@ export { addDayOffset, type IsWorkingDay, MAX_SEARCH_DAYS, workingDaysOf } from "./dayOffset"; export { dueTriggerDate } from "./recordDateTrigger"; +export { + type EventStart, + isTriggeringEvent, + matchEventStarts, + TRIGGERING_EVENTS, + type TriggerEvent, +} from "./eventTrigger"; export { type DayOffset, + type EventTrigger, parseProcessDefinition, + PROCESS_DEFINITION_SCHEMA_VERSION, type ProcessDefinition, + type RecordDateTrigger, type TaskStep, WELCOME_NEW_STARTER, } from "./processDefinition"; @@ -18,6 +28,7 @@ export { removalCancellation, reopenStep, resolveAssignee, + type ResolvedAssignee, RUN_STATUSES, type RunStatus, type RunStep, diff --git a/packages/api/src/modules/processes/domain/processDefinition.test.ts b/packages/api/src/modules/processes/domain/processDefinition.test.ts index 18879a115..3bcd050d6 100644 --- a/packages/api/src/modules/processes/domain/processDefinition.test.ts +++ b/packages/api/src/modules/processes/domain/processDefinition.test.ts @@ -16,21 +16,21 @@ test("refuses an unknown definition schema version", () => { ); }); -test("refuses a record-created trigger until durable events exist", () => { +test("refuses an unbuilt record-created trigger", () => { const definition = welcomeDefinition(); definition.trigger.kind = "on_record_created"; expect(() => parseProcessDefinition(1, definition)).toThrow( - "Trigger kind on_record_created needs durable record events, which are not built yet.", + "Trigger kind on_record_created is not built.", ); }); -test("refuses a record-changed trigger until durable events exist", () => { +test("refuses an unbuilt record-changed trigger", () => { const definition = welcomeDefinition(); definition.trigger.kind = "on_record_changed"; expect(() => parseProcessDefinition(1, definition)).toThrow( - "Trigger kind on_record_changed needs durable record events, which are not built yet.", + "Trigger kind on_record_changed is not built.", ); }); @@ -39,7 +39,7 @@ test("refuses a schedule trigger until it is supported", () => { definition.trigger.kind = "on_schedule"; expect(() => parseProcessDefinition(1, definition)).toThrow( - "Trigger kind on_schedule needs durable record events, which are not built yet.", + "Trigger kind on_schedule is not built.", ); }); @@ -118,6 +118,76 @@ test("refuses a trigger date other than the employee's start date", () => { expect(() => parseProcessDefinition(1, definition)).toThrow(); }); +test.each([ + ["employees.added", "employee"], + ["tasks.task_created", "task"], + ["tasks.task_status_changed", "task"], +])("accepts %s as an event trigger for %s records", (event, recordType) => { + const definition = eventDefinition(event, recordType); + + expect(parseProcessDefinition(1, definition)).toEqual(definition); +}); + +test("refuses an event that cannot trigger a process", () => { + expect(() => parseProcessDefinition(1, eventDefinition("example.happened"))).toThrow( + "Event example.happened cannot trigger a process.", + ); +}); + +test("refuses an event condition without any accepted values", () => { + const definition = eventDefinition("tasks.task_status_changed", "task"); + const trigger = { ...definition.trigger, when: { field: "to", in: [] } }; + + expect(() => parseProcessDefinition(1, { ...definition, trigger })).toThrow(); +}); + +test("refuses an event trigger that runs after commit", () => { + const definition = eventDefinition("employees.added"); + definition.trigger.run = "after_commit"; + + expect(() => parseProcessDefinition(1, definition)).toThrow(); +}); + +test("refuses an event trigger that silently ignores failures", () => { + const definition = eventDefinition("employees.added"); + definition.trigger.faultMode = "silent"; + + expect(() => parseProcessDefinition(1, definition)).toThrow(); +}); + +test("refuses an event trigger without a run mode", () => { + const definition = eventDefinition("employees.added"); + const { kind, event, faultMode } = definition.trigger; + + expect(() => + parseProcessDefinition(1, { + ...definition, + trigger: { kind, event, faultMode }, + }), + ).toThrow(); +}); + +test("refuses a subject differing from the triggering event's record type", () => { + const definition = eventDefinition("tasks.task_created", "employee"); + + expect(() => parseProcessDefinition(1, definition)).toThrow( + "The subject employee differs from the record type task of event tasks.task_created.", + ); +}); + +function eventDefinition(event: string, recordType = "employee") { + return { + ...welcomeDefinition(), + subject: { recordType }, + trigger: { + kind: "on_event", + event, + run: "in_same_transaction", + faultMode: "rollback_transaction", + }, + }; +} + function welcomeDefinition() { return { subject: { recordType: "employee" }, diff --git a/packages/api/src/modules/processes/domain/processDefinition.ts b/packages/api/src/modules/processes/domain/processDefinition.ts index f667cd77e..112e492ca 100644 --- a/packages/api/src/modules/processes/domain/processDefinition.ts +++ b/packages/api/src/modules/processes/domain/processDefinition.ts @@ -1,5 +1,7 @@ import { z } from "zod"; +import { isTriggeringEvent, TRIGGERING_EVENTS, type TriggeringEventType } from "./eventTrigger"; + /** The shape of `process_versions.definition` this code reads; a migration moves older shapes on. */ export const PROCESS_DEFINITION_SCHEMA_VERSION = 1; @@ -40,10 +42,24 @@ export interface RecordDateTrigger { maxDaysLate: number; } +/** + * Starts a run for each record an event is about, inside the transaction that published it: the + * run commits with the change, and a run that fails fails the change. + */ +export interface EventTrigger { + kind: "on_event"; + event: TriggeringEventType; + /** Only events whose payload field holds one of these values. */ + when?: { field: string; in: (string | null)[] }; + run: "in_same_transaction"; + faultMode: "rollback_transaction"; +} + /** What one version of a process does; its steps run in parallel, listed in display order. */ export interface ProcessDefinition { - subject: { recordType: "employee" }; - trigger: RecordDateTrigger; + /** The record a run is about, the trigger's record type. */ + subject: { recordType: "employee" | "task" }; + trigger: RecordDateTrigger | EventTrigger; startableByAdmin: boolean; steps: TaskStep[]; } @@ -56,21 +72,38 @@ export function parseProcessDefinition( if (schemaVersion !== PROCESS_DEFINITION_SCHEMA_VERSION) { throw new Error(`Process definition schema version ${schemaVersion} is not known.`); } - refuseEventTrigger(definition); + refuseUnbuiltTrigger(definition); const parsed: ProcessDefinition = definitionSchema.parse(definition); + refuseSubjectOtherThanTrigger(parsed); refuseRepeatedStepKeys(parsed.steps); - for (const offset of [parsed.trigger.offset, ...parsed.steps.map(({ due }) => due)]) { + const triggerOffsets = parsed.trigger.kind === "on_record_date" ? [parsed.trigger.offset] : []; + for (const offset of [...triggerOffsets, ...parsed.steps.map(({ due }) => due)]) { refuseOffsetBeyondLimit(offset); } return parsed; } -const EVENT_TRIGGER_KINDS = new Set(["on_record_created", "on_record_changed", "on_schedule"]); +const UNBUILT_TRIGGER_KINDS = new Set(["on_record_created", "on_record_changed", "on_schedule"]); -function refuseEventTrigger(definition: unknown): void { - const kind = (definition as { trigger?: { kind?: unknown } } | null)?.trigger?.kind; - if (typeof kind === "string" && EVENT_TRIGGER_KINDS.has(kind)) { - throw new Error(`Trigger kind ${kind} needs durable record events, which are not built yet.`); +function refuseUnbuiltTrigger(definition: unknown): void { + const trigger = (definition as { trigger?: { kind?: unknown; event?: unknown } } | null)?.trigger; + if (typeof trigger?.kind === "string" && UNBUILT_TRIGGER_KINDS.has(trigger.kind)) { + throw new Error(`Trigger kind ${trigger.kind} is not built.`); + } + if (trigger?.kind === "on_event" && !isTriggeringEvent(String(trigger.event))) { + throw new Error(`Event ${String(trigger.event)} cannot trigger a process.`); + } +} + +function refuseSubjectOtherThanTrigger({ subject, trigger }: ProcessDefinition): void { + const [recordType, source] = + trigger.kind === "on_event" + ? [TRIGGERING_EVENTS[trigger.event].recordType, `event ${trigger.event}`] + : [trigger.recordType, `trigger ${trigger.kind}`]; + if (subject.recordType !== recordType) { + throw new Error( + `The subject ${subject.recordType} differs from the record type ${recordType} of ${source}.`, + ); } } @@ -102,15 +135,31 @@ const assigneeRuleSchema: z.ZodType = z.lazy(() => }), ); -const definitionSchema = z.object({ - subject: z.object({ recordType: z.literal("employee") }), - trigger: z.object({ +const triggerSchema = z.discriminatedUnion("kind", [ + z.object({ kind: z.literal("on_record_date"), recordType: z.literal("employee"), dateField: z.literal("startDate"), offset: offsetSchema, maxDaysLate: z.number().int().min(0), }), + z.object({ + kind: z.literal("on_event"), + event: z.enum(Object.keys(TRIGGERING_EVENTS) as [TriggeringEventType]), + when: z + .object({ + field: z.string().trim().min(1), + in: z.array(z.string().nullable()).min(1), + }) + .optional(), + run: z.literal("in_same_transaction"), + faultMode: z.literal("rollback_transaction"), + }), +]); + +const definitionSchema = z.object({ + subject: z.object({ recordType: z.enum(["employee", "task"]) }), + trigger: triggerSchema, startableByAdmin: z.boolean(), steps: z .array( diff --git a/packages/api/src/modules/processes/domain/processRun.ts b/packages/api/src/modules/processes/domain/processRun.ts index 684221ff5..2644cfa46 100644 --- a/packages/api/src/modules/processes/domain/processRun.ts +++ b/packages/api/src/modules/processes/domain/processRun.ts @@ -1,11 +1,16 @@ import { ApplicationError } from "../../../core/applicationError"; import type { CalendarDate } from "../../../utils"; +import type { TriggerEvent } from "./eventTrigger"; import type { AssigneeRule } from "./processDefinition"; export const RUN_STATUSES = ["in_progress", "finished", "cancelled"] as const; export type RunStatus = (typeof RUN_STATUSES)[number]; -export const START_CAUSES = ["started_on_record_date", "started_by_admin"] as const; +export const START_CAUSES = [ + "started_on_record_date", + "started_by_admin", + "started_on_event", +] as const; export type StartCause = (typeof START_CAUSES)[number]; export const CANCEL_CAUSES = [ @@ -49,6 +54,8 @@ export interface ProcessRun { /** The date the trigger fired for; with the process, `startKey` keeps it from starting twice. */ triggerDate: CalendarDate | null; startKey: string | null; + /** The event that started the run, for `started_on_event`; steps read who acted from it. */ + triggerEvent: TriggerEvent | null; /** The day due dates count from. */ startDay: CalendarDate; status: RunStatus; diff --git a/packages/api/src/modules/processes/index.ts b/packages/api/src/modules/processes/index.ts index 10342cb08..a6a1d6ea6 100644 --- a/packages/api/src/modules/processes/index.ts +++ b/packages/api/src/modules/processes/index.ts @@ -1,4 +1,5 @@ import { readSdl } from "../../core/graphql"; +import { PrincipalSecurityContext } from "../../core/security"; import type { ModuleDependencies, ModuleManifest } from "../../core/moduleManifest"; import { type ProcessService, startDueRunsJob } from "./app"; @@ -15,5 +16,7 @@ export function processesModule({ typeDefs: readSdl(import.meta.url, "./graphql/schema.graphql"), resolvers: {}, jobs: startDueRunsJob({ processes, logger }), + eventsBeforeCommit: (tx, events) => + processes.startRunsOnEvents(PrincipalSecurityContext.system(), tx, events), }; } diff --git a/packages/api/src/modules/processes/module.md b/packages/api/src/modules/processes/module.md index 7b49bc92d..e41dea42d 100644 --- a/packages/api/src/modules/processes/module.md +++ b/packages/api/src/modules/processes/module.md @@ -5,7 +5,8 @@ A process gives people tasks when something happens to a record, and follows eac ## Rules - A process is off until an admin turns it on (`MANAGE_WORKSPACE`); turning it on or off appends a history line for record type `process`, which only those who manage the workspace read. Turning it on starts today's due runs at once, and never anyone whose date was before that day. -- What a process does lives in its published version's definition (JSON, read by `parseProcessDefinition` for the `definition_schema_version` it was written in). A published version never changes; a new release ships a new version. The trigger fires on a record's date plus an offset; event and schedule triggers wait for durable record events and are refused. +- What a process does lives in its published version's definition (JSON, read by `parseProcessDefinition` for the `definition_schema_version` it was written in). A published version never changes; a new release ships a new version. The trigger fires on a record's date plus an offset (`on_record_date`, employees only), or on a domain event (`on_event`); schedule triggers are refused. +- An `on_event` trigger names an event listed in `TRIGGERING_EVENTS` (`employees.added`, `tasks.task_created`, `tasks.task_status_changed`), with an optional `when` on one payload field, and runs `in_same_transaction` with `faultMode` `rollback_transaction`: before the change commits, `startRunsOnEvents` starts one run per event and record the event is about, batched per process, skipping a record whose run is in progress or that is gone. The run keeps the event (`trigger_event`); a run that cannot start fails the change. - The daily job (`processes.startDueRuns`, 06:00 London) starts a run for each record whose trigger date is today or up to the trigger's `maxDaysLate` days ago, if it never had a run of that process, one transaction each. It also cancels runs that can no longer run. - An admin starts a run by hand (`create` on `process_run` for the person), whether the process is on or off, unless one is in progress for them. Before the person's start date, the run counts from their start date. - Steps run in parallel. Each step's task goes to its assignee rule's person on the run's start day, else its fallback, with a note when the owner covers a missing manager; with nobody, the step is skipped. Due dates count calendar days, or business days by the person's own working weeks and bank holidays, refusing a year with no known bank holidays. @@ -17,5 +18,5 @@ A process gives people tasks when something happens to a record, and follows eac - **`processes`:** `key`, `name`, `description`, `active_version_id` (one of its own published versions), `turned_on_at` (null while off). - **`process_versions`:** `version`, `status` (`draft` or `published`, one draft per process), `definition_schema_version`, `definition`. -- **`process_runs`:** the version it started with, the record it is about (`record_type`, `record_id`) and its subject (`subject_employee_id`, no foreign key so a removal is noticed), how it started, `trigger_date` and `start_key` (unique per process, so a trigger starts once), `start_day`, `status` (`in_progress`, `finished`, `cancelled`) with its columns. One run in progress per process and record. +- **`process_runs`:** the version it started with, the record it is about (`record_type`, `record_id`: an employee or a task) and its subject (`subject_employee_id`, no foreign key so a removal is noticed; a task's subject, which may be null), how it started, `trigger_date` or `trigger_event` and `start_key` (unique per process, so a trigger starts once), `start_day`, `status` (`in_progress`, `finished`, `cancelled`) with its columns. One run in progress per process and record. - **`process_run_steps`:** one row per step: `status`, `task_id` (set null when the task goes with its assignee), `assignee_note`, `skip_reason`, `finished_at`. diff --git a/packages/api/src/modules/tasks/app/createTask.int.test.ts b/packages/api/src/modules/tasks/app/createTask.int.test.ts index 7e88d4248..950f3a28f 100644 --- a/packages/api/src/modules/tasks/app/createTask.int.test.ts +++ b/packages/api/src/modules/tasks/app/createTask.int.test.ts @@ -1,4 +1,12 @@ -import { employeeServices, expect, OWNER, signedIn, taskService, test } from "#test/support"; +import { + domainEvents, + employeeServices, + expect, + OWNER, + signedIn, + taskService, + test, +} from "#test/support"; test("creates a task needing its assignee and awaited by its asker", async ({ orm, @@ -177,3 +185,78 @@ test("deletes a person's held tasks when the person is deleted", async ({ orm, s message: "No task has this id.", }); }); + +test("publishes the created task's ids and the person who created it", async ({ orm, seed }) => { + const assignee = await seed.employee(); + const asker = await seed.employee(); + const subject = await seed.employee(); + const ctx = signedIn(orm, { employeeId: asker.id, profiles: ["EMPLOYEE"] }); + const { transact, events } = domainEvents(orm); + const tasks = taskService(orm, { transact }); + + const task = await transact((tx) => + tasks.createTask(ctx, tx, { + type: "manual_to_do", + title: "Read the handbook", + assigneeEmployeeId: assignee.id, + askedByEmployeeId: asker.id, + subjectEmployeeId: subject.id, + processSource: { processKey: "onboarding", processRunId: null, stepKey: "handbook" }, + }), + ); + + expect(events).toEqual([ + { + type: "tasks.task_created", + payload: { + taskId: task.id, + assigneeEmployeeId: assignee.id, + askedByEmployeeId: asker.id, + subjectEmployeeId: subject.id, + processKey: "onboarding", + byEmployeeId: asker.id, + }, + }, + ]); +}); + +test("returns bulk-created tasks in input order and publishes each task", async ({ orm, seed }) => { + const assignee = await seed.employee(); + const asker = await seed.employee(); + const ctx = signedIn(orm, { employeeId: asker.id, profiles: ["EMPLOYEE"] }); + const { transact, events } = domainEvents(orm); + const tasks = taskService(orm, { transact }); + + const created = await transact((tx) => + tasks.createTasks(ctx, tx, [ + { + type: "manual_to_do", + title: "Read the handbook", + assigneeEmployeeId: assignee.id, + askedByEmployeeId: asker.id, + }, + { + type: "manual_to_do", + title: "Arrange a meeting", + assigneeEmployeeId: asker.id, + askedByEmployeeId: asker.id, + }, + ]), + ); + + expect(created.map(({ title }) => title)).toEqual(["Read the handbook", "Arrange a meeting"]); + expect(created[0]!.id).not.toBe(created[1]!.id); + expect(events).toEqual( + created.map((task) => ({ + type: "tasks.task_created", + payload: { + taskId: task.id, + assigneeEmployeeId: task.assigneeEmployeeId, + askedByEmployeeId: asker.id, + subjectEmployeeId: null, + processKey: null, + byEmployeeId: asker.id, + }, + })), + ); +}); diff --git a/packages/api/src/modules/tasks/app/createTasks.ts b/packages/api/src/modules/tasks/app/createTasks.ts new file mode 100644 index 000000000..45b40441b --- /dev/null +++ b/packages/api/src/modules/tasks/app/createTasks.ts @@ -0,0 +1,95 @@ +import { invalidInput } from "../../../core/applicationError"; +import type { Tx } from "../../../core/db"; +import { createRecordHistoryService, type CauseKinds } from "../../../core/records"; +import type { SecurityContext } from "../../../core/security"; +import type { CalendarDate } from "../../../utils"; +import { type NewTaskRow, taskRepository } from "../db"; +import type { Task, TaskProcessSource, TaskRecord, TaskType } from "../domain"; + +/** A task a feature, a process step or the demo asks of someone: a person who asked, a process step, or both. */ +export type NewTask = NewTaskFields & + ( + | { askedByEmployeeId: string; processSource?: TaskProcessSource | null } + | { askedByEmployeeId?: null; processSource: TaskProcessSource } + ); + +interface NewTaskFields { + type: TaskType; + title: string; + description?: string | null; + assigneeEmployeeId: string; + subjectEmployeeId?: string | null; + record?: TaskRecord | null; + dueDate?: CalendarDate | null; + /** Why the task exists, for its history; a direct edit when left out. */ + cause?: TaskCause; +} + +/** The cause a task's history line records. */ +export type TaskCause = { kind: keyof CauseKinds; ref?: string | null }; + +/** Inserts the tasks to do, appends their history lines and publishes their creation. */ +export async function createTasks( + ctx: SecurityContext, + tx: Tx, + newTasks: readonly NewTask[], +): Promise { + const created = await taskRepository(tx).insert(newTasks.map(newTaskRow)); + await history.append(ctx, tx, { + recordType: "task", + operation: "CREATED", + cause: DIRECT_EDIT, + records: created.map((task, index) => ({ + recordId: task.id, + cause: newTasks[index]!.cause ?? DIRECT_EDIT, + changes: [ + { field: "title", oldValue: null, newValue: task.title }, + { field: "status", oldValue: null, newValue: task.status }, + ], + })), + }); + for (const task of created) { + tx.publish("tasks.task_created", { + taskId: task.id, + assigneeEmployeeId: task.assigneeEmployeeId, + askedByEmployeeId: task.askedByEmployeeId, + subjectEmployeeId: task.subjectEmployeeId, + processKey: task.processSource?.processKey ?? null, + byEmployeeId: ctx.user?.employeeId ?? null, + }); + } + return created; +} + +function newTaskRow({ title, description = null, ...task }: NewTask): NewTaskRow { + return { + type: task.type, + status: "to_do", + title: requireText(title, "title"), + description: description?.trim() || null, + assigneeEmployeeId: task.assigneeEmployeeId, + askedByEmployeeId: task.askedByEmployeeId ?? null, + subjectEmployeeId: task.subjectEmployeeId ?? null, + processSource: task.processSource ?? null, + record: task.record ?? null, + dueDate: task.dueDate ?? null, + doneAt: null, + doneByEmployeeId: null, + cancelledAt: null, + cancelReason: null, + }; +} + +/** The text trimmed, refused as invalid input when blank. */ +export function requireText(value: string, field: string): string { + const trimmed = value.trim(); + if (!trimmed) { + throw invalidInput([{ field, code: "required" }]); + } + return trimmed; +} + +/** A change a person made directly, the cause of most task history lines. */ +export const DIRECT_EDIT: TaskCause = { kind: "EDITED_DIRECTLY" }; + +const history = createRecordHistoryService(); diff --git a/packages/api/src/modules/tasks/app/index.ts b/packages/api/src/modules/tasks/app/index.ts index 877c704d7..dc4fbf1c9 100644 --- a/packages/api/src/modules/tasks/app/index.ts +++ b/packages/api/src/modules/tasks/app/index.ts @@ -1,3 +1,4 @@ -export { createTaskService, type NewTask, type TaskChange, type TaskService } from "./taskService"; +export type { NewTask } from "./createTasks"; +export { createTaskService, type TaskChange, type TaskService } from "./taskService"; export { taskRecordType } from "./taskRecordType"; export type { MyTasks } from "../domain"; diff --git a/packages/api/src/modules/tasks/app/markDone.int.test.ts b/packages/api/src/modules/tasks/app/markDone.int.test.ts index 0a027e74e..8d3bca866 100644 --- a/packages/api/src/modules/tasks/app/markDone.int.test.ts +++ b/packages/api/src/modules/tasks/app/markDone.int.test.ts @@ -1,4 +1,4 @@ -import { expect, signedIn, taskService, test } from "#test/support"; +import { domainEvents, expect, signedIn, taskService, test } from "#test/support"; test("marks a held task done and shows the asker who completed it", async ({ orm, seed }) => { const assignee = await seed.employee(); @@ -118,3 +118,28 @@ test("reports an unknown task as not found when completing it", async ({ orm, se taskService(orm).markDone(ctx, "00000000-0000-4000-8000-000000000001"), ).rejects.toMatchObject({ code: "NOT_FOUND", message: "No task has this id." }); }); + +test("publishes a status change with the assignee who marked the task done", async ({ + orm, + seed, +}) => { + const assignee = await seed.employee(); + const asker = await seed.employee(); + const task = await seed.task({ assigneeEmployeeId: assignee.id, askedByEmployeeId: asker.id }); + const ctx = signedIn(orm, { employeeId: assignee.id, profiles: ["EMPLOYEE"] }); + const { transact, events } = domainEvents(orm); + + await taskService(orm, { transact }).markDone(ctx, task.id); + + expect(events).toEqual([ + { + type: "tasks.task_status_changed", + payload: { + taskId: task.id, + from: "to_do", + to: "done", + byEmployeeId: assignee.id, + }, + }, + ]); +}); diff --git a/packages/api/src/modules/tasks/app/registrations.ts b/packages/api/src/modules/tasks/app/registrations.ts index edbc98d62..0dd16ef44 100644 --- a/packages/api/src/modules/tasks/app/registrations.ts +++ b/packages/api/src/modules/tasks/app/registrations.ts @@ -1,4 +1,5 @@ // What this module adds to core's shared registries. +import type { TaskStatus } from "../domain"; declare module "../../../core/records" { interface RecordTypes { @@ -7,4 +8,24 @@ declare module "../../../core/records" { } } -export {}; +declare module "../../../core/events" { + interface DomainEvents { + /** A task to do was created; `byEmployeeId` is the person who created it, null for Broccoli. */ + "tasks.task_created": { + taskId: string; + assigneeEmployeeId: string; + askedByEmployeeId: string | null; + subjectEmployeeId: string | null; + /** The process whose step created it, else null. */ + processKey: string | null; + byEmployeeId: string | null; + }; + /** A task's status changed; `byEmployeeId` is the person who changed it, null for Broccoli. */ + "tasks.task_status_changed": { + taskId: string; + from: TaskStatus; + to: TaskStatus; + byEmployeeId: string | null; + }; + } +} diff --git a/packages/api/src/modules/tasks/app/taskService.ts b/packages/api/src/modules/tasks/app/taskService.ts index 4fa69318e..33dd8b353 100644 --- a/packages/api/src/modules/tasks/app/taskService.ts +++ b/packages/api/src/modules/tasks/app/taskService.ts @@ -1,16 +1,16 @@ -import { ApplicationError, invalidInput } from "../../../core/applicationError"; +import { ApplicationError } from "../../../core/applicationError"; import type { Transact, Tx } from "../../../core/db"; import type { ModuleDependencies } from "../../../core/moduleManifest"; import { createRecordHistoryReaderService, createRecordHistoryService, - type CauseKinds, type HistoryLine, } from "../../../core/records"; import type { Connection, PageArgs } from "../../../core/pagination"; import type { SecurityContext } from "../../../core/security"; -import { type CalendarDate, parseUuid } from "../../../utils"; +import { parseUuid } from "../../../utils"; import { taskRepository } from "../db"; +import { createTasks, DIRECT_EDIT, type NewTask, requireText, type TaskCause } from "./createTasks"; import { cancelTask, groupMyTasks, @@ -21,34 +21,9 @@ import { reopenTask, resolvedRecentlySince, type Task, - type TaskProcessSource, - type TaskRecord, - type TaskType, } from "../domain"; import { taskRecordType } from "./taskRecordType"; -/** A task a feature, a process step or the demo asks of someone: a person who asked, a process step, or both. */ -export type NewTask = NewTaskFields & - ( - | { askedByEmployeeId: string; processSource?: TaskProcessSource | null } - | { askedByEmployeeId?: null; processSource: TaskProcessSource } - ); - -interface NewTaskFields { - type: TaskType; - title: string; - description?: string | null; - assigneeEmployeeId: string; - subjectEmployeeId?: string | null; - record?: TaskRecord | null; - dueDate?: CalendarDate | null; - /** Why the task exists, for its history; a direct edit when left out. */ - cause?: TaskCause; -} - -/** The cause a task's history line records. */ -export type TaskCause = { kind: keyof CauseKinds; ref?: string | null }; - /** * Work asked of people: created by features, read and finished by the people who hold or asked * for it. Only they see a task; anyone else gets NOT_FOUND, so its existence does not leak. @@ -56,6 +31,8 @@ export type TaskCause = { kind: keyof CauseKinds; ref?: string | null }; export interface TaskService { /** Trusted, for features and the demo fill: a new task to do in `tx`; the caller authorizes. */ createTask(ctx: SecurityContext, tx: Tx, task: NewTask): Promise; + /** Trusted, for processes: new tasks to do in `tx`, with one insert, returned in the same order. */ + createTasks(ctx: SecurityContext, tx: Tx, tasks: readonly NewTask[]): Promise; /** Trusted, for processes: the tasks among these ids, in `tx`; the caller authorizes. */ readTasksInTransaction(tx: Tx, taskIds: readonly string[]): Promise; /** Trusted, for processes: its assignee marks a feature's task done in `tx`, first change winning. */ @@ -106,40 +83,13 @@ export function createTaskService({ }: Pick): TaskService { const historyReader = createRecordHistoryReaderService(orm, { task: taskRecordType(orm) }); return { - async createTask(ctx, tx, { title, description = null, cause = DIRECT_EDIT, ...task }) { - const created = await taskRepository(tx).insert({ - type: task.type, - status: "to_do", - title: requireText(title, "title"), - description: description?.trim() || null, - assigneeEmployeeId: task.assigneeEmployeeId, - askedByEmployeeId: task.askedByEmployeeId ?? null, - subjectEmployeeId: task.subjectEmployeeId ?? null, - processSource: task.processSource ?? null, - record: task.record ?? null, - dueDate: task.dueDate ?? null, - doneAt: null, - doneByEmployeeId: null, - cancelledAt: null, - cancelReason: null, - }); - await history.append(ctx, tx, { - recordType: "task", - operation: "CREATED", - cause, - records: [ - { - recordId: created.id, - changes: [ - { field: "title", oldValue: null, newValue: created.title }, - { field: "status", oldValue: null, newValue: created.status }, - ], - }, - ], - }); - return created; + async createTask(ctx, tx, task) { + const [created] = await createTasks(ctx, tx, [task]); + return created!; }, + createTasks, + readTasksInTransaction: (tx, taskIds) => taskRepository(tx).findByIds(taskIds), async markDoneInTransaction(ctx, tx, taskId) { @@ -265,6 +215,7 @@ async function changeLockedTask( return { task: saved, historyLine: await appendStatusChange(ctx, tx, task, saved) }; } +/** Appends the status change to the task's history and publishes it. */ async function appendStatusChange( ctx: SecurityContext, tx: Tx, @@ -272,6 +223,12 @@ async function appendStatusChange( after: Task, cause: TaskCause = DIRECT_EDIT, ): Promise { + tx.publish("tasks.task_status_changed", { + taskId: after.id, + from: before.status, + to: after.status, + byEmployeeId: ctx.user?.employeeId ?? null, + }); const [line] = await history.append(ctx, tx, { recordType: "task", operation: "UPDATED", @@ -312,20 +269,11 @@ function refuseFeatureTask(task: Task): void { } } -function requireText(value: string, field: string): string { - const trimmed = value.trim(); - if (!trimmed) { - throw invalidInput([{ field, code: "required" }]); - } - return trimmed; -} - function notFound(): ApplicationError { return new ApplicationError("NOT_FOUND", "No task has this id."); } const history = createRecordHistoryService(); -const DIRECT_EDIT: TaskCause = { kind: "EDITED_DIRECTLY" }; const MARK_DONE_REFUSAL = "Only the person who holds this task can mark it done."; const REOPEN_REFUSAL = "Only the person who holds this task can reopen it."; diff --git a/packages/api/src/modules/tasks/db/taskRepository.ts b/packages/api/src/modules/tasks/db/taskRepository.ts index ea82f86a1..c7c70bbce 100644 --- a/packages/api/src/modules/tasks/db/taskRepository.ts +++ b/packages/api/src/modules/tasks/db/taskRepository.ts @@ -9,7 +9,8 @@ export type NewTaskRow = Omit; /** Stored tasks. */ export interface TaskRepository { - insert(task: NewTaskRow): Promise; + /** Inserts the tasks in one statement; returns them in the same order. */ + insert(tasks: readonly NewTaskRow[]): Promise; findById(id: string): Promise; findByIds(ids: readonly string[]): Promise; /** The task with this id, or null, locked until the transaction ends, so the first change wins. */ @@ -28,9 +29,10 @@ export interface TaskRepository { export function taskRepository(db: Orm | Tx): TaskRepository { const orm = ormOf(db); return { - async insert(task) { - const [row] = await orm.insert(tasks).values(toRow(task)).returning(); - return toTask(row!); + async insert(newTasks) { + if (newTasks.length === 0) return []; + const rows = await orm.insert(tasks).values(newTasks.map(toRow)).returning(); + return rows.map(toTask); }, async findById(id) { diff --git a/packages/api/test/support/domainEvents.ts b/packages/api/test/support/domainEvents.ts new file mode 100644 index 000000000..2b9c9bc10 --- /dev/null +++ b/packages/api/test/support/domainEvents.ts @@ -0,0 +1,20 @@ +import { type Orm, transactor } from "../../src/core/db"; +import { createEventBus, type DomainEvent } from "../../src/core/events"; +import { createLogger } from "../../src/core/logger"; + +/** Task events delivered after the transaction commits. */ +export function domainEvents(orm: Orm) { + const events: DomainEvent[] = []; + const eventBus = createEventBus(createLogger("silent")); + eventBus.subscribe({ + "tasks.task_created": (payload) => { + events.push({ type: "tasks.task_created", payload }); + return Promise.resolve(); + }, + "tasks.task_status_changed": (payload) => { + events.push({ type: "tasks.task_status_changed", payload }); + return Promise.resolve(); + }, + }); + return { transact: transactor(orm, eventBus), events }; +} diff --git a/packages/api/test/support/index.ts b/packages/api/test/support/index.ts index fab9472f8..373491cca 100644 --- a/packages/api/test/support/index.ts +++ b/packages/api/test/support/index.ts @@ -78,7 +78,8 @@ export { passwordResetEmail, } from "./services"; export type { EmployeeServices } from "./services"; -export { processService } from "./processService"; +export { processService, transactWithProcesses } from "./processService"; +export { domainEvents } from "./domainEvents"; export { employeeSetup, fileStorage, diff --git a/packages/api/test/support/processService.ts b/packages/api/test/support/processService.ts index 58fbdaf26..8ee2f28ad 100644 --- a/packages/api/test/support/processService.ts +++ b/packages/api/test/support/processService.ts @@ -1,6 +1,7 @@ -import type { Orm } from "../../src/core/db"; +import { createBeforeCommitBus, type Orm, type Transact, transactor } from "../../src/core/db"; +import { createEventBus } from "../../src/core/events"; import { createLogger } from "../../src/core/logger"; -import { createProcessService } from "../../src/modules/processes"; +import { createProcessService, processesModule } from "../../src/modules/processes"; import { type CalendarDate, todayInLondon } from "../../src/utils"; import { bankHolidayService } from "./bankHolidayService"; import { taskService } from "./services"; @@ -9,14 +10,33 @@ import { testTransactor } from "./testTransactor"; /** The process service with real tasks, offline bank holidays and an injectable day, at noon. */ export function processService( orm: Orm, - { today = todayInLondon }: { today?: () => CalendarDate } = {}, + { + today = todayInLondon, + transact = testTransactor(orm), + }: { + today?: () => CalendarDate; + transact?: Transact; + } = {}, ) { return createProcessService({ orm, - transact: testTransactor(orm), + transact, logger: createLogger("silent"), - tasks: taskService(orm), + tasks: taskService(orm, { transact }), bankHolidays: bankHolidayService(), now: () => new Date(`${today()}T12:00:00Z`), }); } + +/** Transactions whose events start processes before the commit, using the supplied day. */ +export function transactWithProcesses( + orm: Orm, + { today }: { today: () => CalendarDate }, +): Transact { + const logger = createLogger("silent"); + const beforeCommit = createBeforeCommitBus(); + const transact = transactor(orm, createEventBus(logger), beforeCommit); + const processes = processService(orm, { today, transact }); + beforeCommit.subscribe(processesModule({ logger, processes }).eventsBeforeCommit!); + return transact; +} diff --git a/packages/api/test/support/seedProcess.ts b/packages/api/test/support/seedProcess.ts new file mode 100644 index 000000000..5834aa669 --- /dev/null +++ b/packages/api/test/support/seedProcess.ts @@ -0,0 +1,28 @@ +import type { Orm } from "../../src/core/db"; +import { processRepository } from "../../src/modules/processes/db"; +import type { ProcessDefinition } from "../../src/modules/processes/domain"; +import { testTransactor } from "./testTransactor"; + +/** A process with its first published version active, without invoking use cases or events. */ +export function seedProcess( + orm: Orm, + { + key, + definition, + turnedOn = true, + }: { + key: string; + definition: ProcessDefinition; + turnedOn?: boolean; + }, +): Promise<{ id: string; key: string }> { + return testTransactor(orm)((tx) => + processRepository(tx).insertProcess({ + key, + name: key, + description: "Test process", + definition, + turnedOnAt: turnedOn ? new Date() : null, + }), + ); +} diff --git a/packages/api/test/support/seedTask.ts b/packages/api/test/support/seedTask.ts index b1c13c5e9..11f009c8d 100644 --- a/packages/api/test/support/seedTask.ts +++ b/packages/api/test/support/seedTask.ts @@ -6,21 +6,24 @@ export function seedTask( orm: Orm, overrides: Pick & Partial, ) { - return testTransactor(orm)((tx) => - taskRepository(tx).insert({ - type: "manual_to_do", - status: "to_do", - description: null, - askedByEmployeeId: null, - subjectEmployeeId: null, - processSource: null, - record: null, - dueDate: null, - doneAt: null, - doneByEmployeeId: null, - cancelledAt: null, - cancelReason: null, - ...overrides, - }), - ); + return testTransactor(orm)(async (tx) => { + const [task] = await taskRepository(tx).insert([ + { + type: "manual_to_do", + status: "to_do", + description: null, + askedByEmployeeId: null, + subjectEmployeeId: null, + processSource: null, + record: null, + dueDate: null, + doneAt: null, + doneByEmployeeId: null, + cancelledAt: null, + cancelReason: null, + ...overrides, + }, + ]); + return task!; + }); } diff --git a/packages/api/test/support/services.ts b/packages/api/test/support/services.ts index 06a7d70cc..88f954e1f 100644 --- a/packages/api/test/support/services.ts +++ b/packages/api/test/support/services.ts @@ -39,7 +39,6 @@ import { import { createImportServices } from "../../src/modules/imports"; import { createExternalAgentServices } from "../../src/modules/externalAgents"; import { createSystemService } from "../../src/modules/system"; -import { createTaskService } from "../../src/modules/tasks"; import { parseCalendarDate } from "../../src/utils"; import { createSlackConnectionService } from "../../src/modules/slack"; import { fakeMail } from "./fakeMail"; @@ -62,8 +61,7 @@ export function companyService( return createCompanyService({ orm, transact: testTransactor(orm), logger, brandLookup }); } -export const taskService = (orm: Orm) => - createTaskService({ orm, transact: testTransactor(orm), logger: createLogger("silent") }); +export { taskService } from "./taskService"; /** Every employee service a person's actions reach, together. */ export type EmployeeServices = EmployeeService & diff --git a/packages/api/test/support/taskService.ts b/packages/api/test/support/taskService.ts new file mode 100644 index 000000000..52692b6ba --- /dev/null +++ b/packages/api/test/support/taskService.ts @@ -0,0 +1,11 @@ +import type { Orm, Transact } from "../../src/core/db"; +import { createLogger } from "../../src/core/logger"; +import { createTaskService } from "../../src/modules/tasks"; +import { testTransactor } from "./testTransactor"; + +export function taskService( + orm: Orm, + { transact = testTransactor(orm) }: { transact?: Transact } = {}, +) { + return createTaskService({ orm, transact, logger: createLogger("silent") }); +} diff --git a/packages/api/test/support/testSeed.ts b/packages/api/test/support/testSeed.ts index 83adaf07d..820645818 100644 --- a/packages/api/test/support/testSeed.ts +++ b/packages/api/test/support/testSeed.ts @@ -20,6 +20,7 @@ import { defaultTermsId, insertTerms, leaveTerms, moveToTerms } from "./seedTerm import { seedUser } from "./seedUser"; import { seedAgentConnection } from "./seedAgentConnection"; import { seedTask } from "./seedTask"; +import { seedProcess } from "./seedProcess"; import { testTransactor } from "./testTransactor"; type SeedEmployee = Partial< @@ -186,6 +187,9 @@ export class TestSeed { agentConnection = (options: Parameters[1]) => seedAgentConnection(this.orm, options); + /** A process with its first version published and active. */ + process = (options: Parameters[1]) => seedProcess(this.orm, options); + /** The id of the seeded "England and Wales" terms, which every new employee joins. */ defaultTermsId = () => defaultTermsId(this.orm); From 6c4443e27ee111f58a5a7a65137979766b251dca Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 12:10:05 +0100 Subject: [PATCH 02/14] fix(processes): name the process and events when a triggered start fails the change --- packages/api/src/core/db/transaction.ts | 3 +- .../modules/processes/app/eventRunService.ts | 50 ++++++++++++------- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/packages/api/src/core/db/transaction.ts b/packages/api/src/core/db/transaction.ts index 9654d7073..316bad30f 100644 --- a/packages/api/src/core/db/transaction.ts +++ b/packages/api/src/core/db/transaction.ts @@ -97,8 +97,9 @@ async function handBeforeCommit(transaction: Transaction, beforeCommit: BeforeCo let handed = 0; for (let round = 0; handed < transaction.events.length; round += 1) { if (round === MAX_BEFORE_COMMIT_ROUNDS) { + const types = [...new Set(transaction.events.slice(handed).map(({ type }) => type))]; throw new Error( - `Events kept publishing more events before the commit after ${MAX_BEFORE_COMMIT_ROUNDS} rounds.`, + `Events kept publishing more events before the commit after ${MAX_BEFORE_COMMIT_ROUNDS} rounds. The last round published ${types.join(", ")}.`, ); } const events = transaction.events.slice(handed); diff --git a/packages/api/src/modules/processes/app/eventRunService.ts b/packages/api/src/modules/processes/app/eventRunService.ts index 48d4a6548..b61c0bef6 100644 --- a/packages/api/src/modules/processes/app/eventRunService.ts +++ b/packages/api/src/modules/processes/app/eventRunService.ts @@ -33,24 +33,8 @@ export function createEventRunService(dependencies: EventRunDependencies): Event const processes = await processRepository(tx).lockProcessesStartedBy([ ...new Set(triggering.map(({ type }) => type)), ]); - const definitions = new Map( - processes.map(({ id, activeVersion }) => [ - id, - parseProcessDefinition(activeVersion.schemaVersion, activeVersion.definition), - ]), - ); - const starts = matchEventStarts( - processes.map(({ id }) => ({ processId: id, trigger: definitions.get(id)!.trigger })), - triggering, - ); for (const process of processes) { - const own = starts.filter(({ processId }) => processId === process.id); - if (own.length === 0) continue; - await startProcessRuns(dependencies, tx, { - process, - definition: definitions.get(process.id)!, - starts: own, - }); + await startRunsOf(dependencies, tx, process, triggering); } }, }; @@ -66,6 +50,38 @@ interface EventRunDependencies { type Start = EventStart; +/** Starts the runs one process's trigger matches; a failure is logged with the process and rethrown. */ +async function startRunsOf( + dependencies: EventRunDependencies, + tx: Tx, + process: StoredProcess, + events: readonly PublishedEvent[], +): Promise { + try { + const { activeVersion } = process; + const definition = parseProcessDefinition( + activeVersion.schemaVersion, + activeVersion.definition, + ); + const starts = matchEventStarts( + [{ processId: process.id, trigger: definition.trigger }], + events, + ); + if (starts.length > 0) + await startProcessRuns(dependencies, tx, { process, definition, starts }); + } catch (error) { + dependencies.logger.error( + { + processKey: process.key, + eventIds: events.map(({ id }) => id), + kind: (error as Error | undefined)?.constructor?.name, + }, + "Process runs not started, so the change that triggered them rolls back", + ); + throw error; + } +} + /** * Starts one process's runs in waves, each record at most once a wave, so a second event for a * record finds the first one's run when it is still in progress. From 61a310b3210ac23c8c335669557d0c793a95fc7f Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 12:14:37 +0100 Subject: [PATCH 03/14] refactor(processes): declare event run dependencies before their factory --- .../src/modules/processes/app/eventRunService.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/api/src/modules/processes/app/eventRunService.ts b/packages/api/src/modules/processes/app/eventRunService.ts index b61c0bef6..3caabdbef 100644 --- a/packages/api/src/modules/processes/app/eventRunService.ts +++ b/packages/api/src/modules/processes/app/eventRunService.ts @@ -24,6 +24,14 @@ export interface EventRunService { startRunsOnEvents(ctx: SecurityContext, tx: Tx, events: readonly PublishedEvent[]): Promise; } +interface EventRunDependencies { + logger: Logger; + tasks: TaskService; + starter: RunStarter; + /** Today in London: the start day of the runs. */ + today: () => CalendarDate; +} + export function createEventRunService(dependencies: EventRunDependencies): EventRunService { return { async startRunsOnEvents(ctx, tx, events) { @@ -40,14 +48,6 @@ export function createEventRunService(dependencies: EventRunDependencies): Event }; } -interface EventRunDependencies { - logger: Logger; - tasks: TaskService; - starter: RunStarter; - /** Today in London: the start day of the runs. */ - today: () => CalendarDate; -} - type Start = EventStart; /** Starts the runs one process's trigger matches; a failure is logged with the process and rethrown. */ From fb6cd147f2c52a5884430a752703033298cbcb77 Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 13:40:10 +0100 Subject: [PATCH 04/14] fix(processes): start event runs without process locks, in insert batches, keeping each run's own event --- .../modules/processes/app/eventRunService.ts | 38 ++++------- .../modules/processes/app/registrations.ts | 7 ++ .../app/startRunsOnEvents.int.test.ts | 34 ++-------- .../app/startRunsOnEventsAtScale.int.test.ts | 68 +++++++++++++++++++ .../modules/processes/db/processRepository.ts | 49 +++++++------ .../modules/processes/domain/eventTrigger.ts | 30 ++++++-- .../api/src/modules/processes/domain/index.ts | 1 + packages/api/src/modules/processes/index.ts | 7 ++ .../src/modules/tasks/db/taskRepository.ts | 15 ++-- packages/api/test/support/index.ts | 1 + packages/api/test/support/seedProcess.ts | 28 ++++++++ 11 files changed, 195 insertions(+), 83 deletions(-) create mode 100644 packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts diff --git a/packages/api/src/modules/processes/app/eventRunService.ts b/packages/api/src/modules/processes/app/eventRunService.ts index 3caabdbef..883787d4e 100644 --- a/packages/api/src/modules/processes/app/eventRunService.ts +++ b/packages/api/src/modules/processes/app/eventRunService.ts @@ -6,6 +6,7 @@ import type { CalendarDate } from "../../../utils"; import type { TaskService } from "../../tasks"; import { processRepository, type StoredProcess } from "../db"; import { + eventAboutRecord, type EventStart, isTriggeringEvent, matchEventStarts, @@ -38,7 +39,7 @@ export function createEventRunService(dependencies: EventRunDependencies): Event ctx.requireSystem(); const triggering = events.filter(({ type }) => isTriggeringEvent(type)); if (triggering.length === 0) return; - const processes = await processRepository(tx).lockProcessesStartedBy([ + const processes = await processRepository(tx).findProcessesStartedBy([ ...new Set(triggering.map(({ type }) => type)), ]); for (const process of processes) { @@ -119,12 +120,18 @@ async function startProcessRuns( startCause: "started_on_event", startedByEmployeeId: null, triggerDate: null, - triggerEvent: event, + triggerEvent: eventAboutRecord(event, recordId), startKey: `on_event:${event.id}:${recordId}`, startDay: today(), })), ); - if (started.length > 0) logStarted(logger, process, started, ready); + if (started.length > 0) { + tx.publish("processes.runs_started_on_event", { + processKey: process.key, + runIds: started.map(({ id }) => id), + eventIds: [...new Set(ready.map(({ event }) => event.id))], + }); + } } } @@ -168,12 +175,11 @@ async function readRecordSubjects( /** The starts split so each record appears at most once a wave, keeping their order. */ function wavesOf(starts: readonly Start[]): Start[][] { const waves: Start[][] = []; + const seen = new Map(); for (const start of starts) { - const wave = waves.find((candidate) => - candidate.every(({ recordId }) => recordId !== start.recordId), - ); - if (wave) wave.push(start); - else waves.push([start]); + const wave = seen.get(start.recordId) ?? 0; + seen.set(start.recordId, wave + 1); + (waves[wave] ??= []).push(start); } return waves; } @@ -189,19 +195,3 @@ function logSkip( "Process run not started for an event", ); } - -function logStarted( - logger: Logger, - process: StoredProcess, - runs: readonly { id: string }[], - starts: readonly Start[], -): void { - logger.info( - { - processKey: process.key, - runIds: runs.map(({ id }) => id), - eventIds: [...new Set(starts.map(({ event }) => event.id))], - }, - "Process runs started with the change that triggered them", - ); -} diff --git a/packages/api/src/modules/processes/app/registrations.ts b/packages/api/src/modules/processes/app/registrations.ts index e7c68c9a7..2d9ac7859 100644 --- a/packages/api/src/modules/processes/app/registrations.ts +++ b/packages/api/src/modules/processes/app/registrations.ts @@ -18,4 +18,11 @@ declare module "../../../core/jobs" { } } +declare module "../../../core/events" { + interface DomainEvents { + /** Runs of the process started with the change whose events triggered them, now committed. */ + "processes.runs_started_on_event": { processKey: string; runIds: string[]; eventIds: string[] }; + } +} + export {}; diff --git a/packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts b/packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts index 9b825e95c..c24b4853a 100644 --- a/packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts +++ b/packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts @@ -10,10 +10,9 @@ import { taskService, test, transactWithProcesses, + eventProcess, } from "#test/support"; -import type { ProcessDefinition } from "../domain"; - test("adding a person commits exactly one process run and a task for their manager", async ({ orm, seed, @@ -99,6 +98,10 @@ test("one event for three people starts three runs", async ({ orm, seed }) => { const runs = await processService(orm).listRuns(ADMIN, key); expect(runs.map(({ record }) => record.recordId).sort()).toEqual([...ids].sort()); expect(new Set(runs.map(({ triggerEvent }) => triggerEvent!.id)).size).toBe(1); + for (const run of runs) { + expect(run.triggerEvent!.type).toBe("employees.added"); + expect(run.triggerEvent!.payload).toEqual({ employeeIds: [run.record.recordId] }); + } }); test("three events in one transaction start three runs", async ({ orm, seed }) => { @@ -311,30 +314,3 @@ test("a matching event during a run in progress is skipped and the save commits" }); const today = () => calendarDate("2026-12-03"); - -function eventProcess( - event: "employees.added" | "tasks.task_created" | "tasks.task_status_changed", - when?: { field: string; in: (string | null)[] }, -): ProcessDefinition { - return { - subject: { recordType: event === "employees.added" ? "employee" : "task" }, - trigger: { - kind: "on_event", - event, - run: "in_same_transaction", - faultMode: "rollback_transaction", - ...(when && { when }), - }, - startableByAdmin: false, - steps: [ - { - key: "follow_up", - kind: "task", - title: "Follow up", - description: null, - assignee: { kind: "subject" }, - due: { amount: 0, unit: "calendar_days" }, - }, - ], - }; -} diff --git a/packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts b/packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts new file mode 100644 index 000000000..94c8bf0c8 --- /dev/null +++ b/packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts @@ -0,0 +1,68 @@ +import { sql } from "drizzle-orm"; + +import { + ADMIN, + calendarDate, + eventProcess, + expect, + processService, + test, + transactWithProcesses, +} from "#test/support"; + +import { ormOf } from "../../../core/db"; +import { processRepository } from "../db"; + +test("one event for more people than one insert can hold starts a run for each", async ({ + orm, + seed, +}) => { + const people = await seed.employees(5_000); + const definition = eventProcess("employees.added"); + definition.steps.push({ ...definition.steps[0]!, key: "second_follow_up" }); + const { key } = await seed.process({ key: "welcome", definition }); + const transact = transactWithProcesses(orm, { today }); + + await transact((tx) => { + tx.publish("employees.added", { employeeIds: people.map(({ id }) => id) }); + return Promise.resolve(); + }); + + expect(await processService(orm).listRuns(ADMIN, key)).toHaveLength(5_000); +}); + +test( + "an event starts its run while another transaction holds the process lock", + { timeout: 5_000 }, + async ({ orm, seed, transact }) => { + const employee = await seed.employee(); + const process = await seed.process({ + key: "welcome", + definition: eventProcess("employees.added"), + }); + const locked = Promise.withResolvers(); + const release = Promise.withResolvers(); + const holding = transact(async (tx) => { + await processRepository(tx).lockProcess(process.id); + locked.resolve(); + await release.promise; + }); + + try { + await Promise.race([locked.promise, holding]); + await transactWithProcesses(orm, { today })(async (tx) => { + await ormOf(tx).execute(sql`set local lock_timeout = '2s'`); + tx.publish("employees.added", { employeeIds: [employee.id] }); + }); + } finally { + release.resolve(); + await holding; + } + + expect(await processService(orm).listRuns(ADMIN, process.key)).toMatchObject([ + { record: { recordType: "employee", recordId: employee.id } }, + ]); + }, +); + +const today = () => calendarDate("2026-12-03"); diff --git a/packages/api/src/modules/processes/db/processRepository.ts b/packages/api/src/modules/processes/db/processRepository.ts index 87ac9200d..0d8f4cbdd 100644 --- a/packages/api/src/modules/processes/db/processRepository.ts +++ b/packages/api/src/modules/processes/db/processRepository.ts @@ -35,20 +35,20 @@ export interface ProcessRepository { findProcessesByIds(ids: readonly string[]): Promise; /** A version's stored definition, not yet parsed; null when no version has the id. */ findVersion(id: string): Promise<{ schemaVersion: number; definition: unknown } | null>; - /** The process, locked until the transaction ends, so every way of starting a run takes turns. */ - lockProcess(id: string): Promise; - /** Every process, locked until the transaction ends, in id order. */ - lockAllProcesses(): Promise; /** - * The turned-on processes whose active version starts on one of these events, share-locked until - * the transaction ends: starts from concurrent changes never wait for each other, while turning - * a process off or a reset waits for them. + * The process, locked until the transaction ends, so the daily job, an admin's start, step + * commands and resets take turns. The lock leaves new runs' foreign keys free, so a change that + * starts a run from an event never waits for it. */ - lockProcessesStartedBy(eventTypes: readonly string[]): Promise; + lockProcess(id: string): Promise; + /** Every process, locked as `lockProcess` does, in id order. */ + lockAllProcesses(): Promise; + /** The turned-on processes whose active version starts on one of these events, unlocked. */ + findProcessesStartedBy(eventTypes: readonly string[]): Promise; /** A new process with its first version, published and active; for tests and seeds. */ insertProcess(process: NewProcessRow): Promise<{ id: string; key: string }>; setTurnedOnAt(id: string, turnedOnAt: Date | null): Promise; - /** Inserts the runs in one statement; returns them in the same order. */ + /** Inserts the runs in batches; returns them in the same order. */ insertRuns(runs: readonly NewRunRow[]): Promise; insertSteps(steps: readonly RunStep[]): Promise; findRun(id: string): Promise; @@ -129,14 +129,18 @@ export function processRepository(db: Orm | Tx): ProcessRepository { .select({ id: processes.id }) .from(processes) .where(eq(processes.id, id)) - .for("update"); + .for("no key update"); }, async lockAllProcesses() { - await orm.select({ id: processes.id }).from(processes).orderBy(processes.id).for("update"); + await orm + .select({ id: processes.id }) + .from(processes) + .orderBy(processes.id) + .for("no key update"); }, - async lockProcessesStartedBy(eventTypes) { + async findProcessesStartedBy(eventTypes) { const rows = await selectProcesses() .where( and( @@ -145,8 +149,7 @@ export function processRepository(db: Orm | Tx): ProcessRepository { sql`${processVersions.definition} -> 'trigger' ->> 'event' = any(${sql.param([...eventTypes])}::text[])`, ), ) - .orderBy(processes.id) - .for("share", { of: processes }); + .orderBy(processes.id); return rows.map(toStoredProcess); }, @@ -177,15 +180,18 @@ export function processRepository(db: Orm | Tx): ProcessRepository { }, async insertRuns(runs) { - if (runs.length === 0) return []; - return await orm - .insert(processRuns) - .values([...runs]) - .returning(); + const inserted: ProcessRun[] = []; + for (let start = 0; start < runs.length; start += INSERT_BATCH_SIZE) { + const batch = runs.slice(start, start + INSERT_BATCH_SIZE); + inserted.push(...(await orm.insert(processRuns).values(batch).returning())); + } + return inserted; }, async insertSteps(steps) { - if (steps.length > 0) await orm.insert(processRunSteps).values([...steps]); + for (let start = 0; start < steps.length; start += INSERT_BATCH_SIZE) { + await orm.insert(processRunSteps).values(steps.slice(start, start + INSERT_BATCH_SIZE)); + } }, async findRun(id) { @@ -298,6 +304,9 @@ async function findRecordsWithRunsWhere( return new Set(rows.map(({ recordId }) => recordId)); } +/** Rows per insert, well below Postgres's limit of 65,535 parameters. */ +const INSERT_BATCH_SIZE = 1_000; + type ProcessWithVersionRow = { process: typeof processes.$inferSelect; version: StoredProcess["activeVersion"]; diff --git a/packages/api/src/modules/processes/domain/eventTrigger.ts b/packages/api/src/modules/processes/domain/eventTrigger.ts index d8d1e6779..8e66d8017 100644 --- a/packages/api/src/modules/processes/domain/eventTrigger.ts +++ b/packages/api/src/modules/processes/domain/eventTrigger.ts @@ -2,22 +2,27 @@ import type { DomainEvents } from "../../../core/events"; import type { EventTrigger, ProcessDefinition } from "./processDefinition"; /** - * The events that may start a process, with the record type each is about and the ids of those - * records in its payload: one run per record. + * The events that may start a process, with the record type each is about, the ids of those + * records in its payload (one run per record), and the payload a run keeps for its own record. */ // shortcut: the list lives here; move it to the publishing modules' registrations when modules become dynamic. export const TRIGGERING_EVENTS = { "employees.added": { recordType: "employee", recordIds: ({ employeeIds }: DomainEvents["employees.added"]) => employeeIds, + payloadFor: (_: DomainEvents["employees.added"], employeeId: string) => ({ + employeeIds: [employeeId], + }), }, "tasks.task_created": { recordType: "task", recordIds: ({ taskId }: DomainEvents["tasks.task_created"]) => [taskId], + payloadFor: (payload: DomainEvents["tasks.task_created"]) => payload, }, "tasks.task_status_changed": { recordType: "task", recordIds: ({ taskId }: DomainEvents["tasks.task_status_changed"]) => [taskId], + payloadFor: (payload: DomainEvents["tasks.task_status_changed"]) => payload, }, } as const; @@ -54,6 +59,14 @@ export function matchEventStarts( ); } +/** The event as it concerns one of its records, as that record's run keeps it. */ +export function eventAboutRecord( + { id, type, payload }: TriggerEvent, + recordId: string, +): TriggerEvent { + return { id, type, payload: entryOf(type).payloadFor(payload, recordId) }; +} + export function isTriggeringEvent(type: string): type is TriggeringEventType { return Object.hasOwn(TRIGGERING_EVENTS, type); } @@ -66,8 +79,13 @@ function isTriggeredBy(trigger: EventTrigger, { type, payload }: TriggerEvent): } function recordIdsOf({ type, payload }: TriggerEvent): readonly string[] { - if (!isTriggeringEvent(type)) return []; - // Each entry reads its own event's payload, which `type` names. - const { recordIds } = TRIGGERING_EVENTS[type] as { recordIds: (payload: unknown) => string[] }; - return recordIds(payload); + return isTriggeringEvent(type) ? entryOf(type).recordIds(payload) : []; +} + +/** An event's entry, typed for any payload: each entry reads its own event's, which `type` names. */ +function entryOf(type: string) { + return TRIGGERING_EVENTS[type as TriggeringEventType] as { + recordIds: (payload: unknown) => string[]; + payloadFor: (payload: unknown, recordId: string) => unknown; + }; } diff --git a/packages/api/src/modules/processes/domain/index.ts b/packages/api/src/modules/processes/domain/index.ts index 64893cfd1..0ce6ed2ff 100644 --- a/packages/api/src/modules/processes/domain/index.ts +++ b/packages/api/src/modules/processes/domain/index.ts @@ -1,6 +1,7 @@ export { addDayOffset, type IsWorkingDay, MAX_SEARCH_DAYS, workingDaysOf } from "./dayOffset"; export { dueTriggerDate } from "./recordDateTrigger"; export { + eventAboutRecord, type EventStart, isTriggeringEvent, matchEventStarts, diff --git a/packages/api/src/modules/processes/index.ts b/packages/api/src/modules/processes/index.ts index a6a1d6ea6..e425d43df 100644 --- a/packages/api/src/modules/processes/index.ts +++ b/packages/api/src/modules/processes/index.ts @@ -16,6 +16,13 @@ export function processesModule({ typeDefs: readSdl(import.meta.url, "./graphql/schema.graphql"), resolvers: {}, jobs: startDueRunsJob({ processes, logger }), + events: { + // Logged once the change commits, never for runs a rollback dropped. + "processes.runs_started_on_event": (started) => + Promise.resolve( + logger.info(started, "Process runs started with the change that triggered them"), + ), + }, eventsBeforeCommit: (tx, events) => processes.startRunsOnEvents(PrincipalSecurityContext.system(), tx, events), }; diff --git a/packages/api/src/modules/tasks/db/taskRepository.ts b/packages/api/src/modules/tasks/db/taskRepository.ts index c7c70bbce..c6934e1bd 100644 --- a/packages/api/src/modules/tasks/db/taskRepository.ts +++ b/packages/api/src/modules/tasks/db/taskRepository.ts @@ -9,7 +9,7 @@ export type NewTaskRow = Omit; /** Stored tasks. */ export interface TaskRepository { - /** Inserts the tasks in one statement; returns them in the same order. */ + /** Inserts the tasks in batches; returns them in the same order. */ insert(tasks: readonly NewTaskRow[]): Promise; findById(id: string): Promise; findByIds(ids: readonly string[]): Promise; @@ -30,9 +30,13 @@ export function taskRepository(db: Orm | Tx): TaskRepository { const orm = ormOf(db); return { async insert(newTasks) { - if (newTasks.length === 0) return []; - const rows = await orm.insert(tasks).values(newTasks.map(toRow)).returning(); - return rows.map(toTask); + const inserted: Task[] = []; + for (let start = 0; start < newTasks.length; start += INSERT_BATCH_SIZE) { + const batch = newTasks.slice(start, start + INSERT_BATCH_SIZE); + const rows = await orm.insert(tasks).values(batch.map(toRow)).returning(); + inserted.push(...rows.map(toTask)); + } + return inserted; }, async findById(id) { @@ -94,6 +98,9 @@ export function taskRepository(db: Orm | Tx): TaskRepository { type TaskRow = typeof tasks.$inferSelect; +/** Rows per insert, well below Postgres's limit of 65,535 parameters. */ +const INSERT_BATCH_SIZE = 1_000; + function toRow({ processSource, record, ...task }: NewTaskRow) { return { ...task, diff --git a/packages/api/test/support/index.ts b/packages/api/test/support/index.ts index 373491cca..7bfcf1698 100644 --- a/packages/api/test/support/index.ts +++ b/packages/api/test/support/index.ts @@ -79,6 +79,7 @@ export { } from "./services"; export type { EmployeeServices } from "./services"; export { processService, transactWithProcesses } from "./processService"; +export { eventProcess } from "./seedProcess"; export { domainEvents } from "./domainEvents"; export { employeeSetup, diff --git a/packages/api/test/support/seedProcess.ts b/packages/api/test/support/seedProcess.ts index 5834aa669..655c540e0 100644 --- a/packages/api/test/support/seedProcess.ts +++ b/packages/api/test/support/seedProcess.ts @@ -26,3 +26,31 @@ export function seedProcess( }), ); } + +/** A process on `event`, with one task step for its subject, due the day it starts. */ +export function eventProcess( + event: "employees.added" | "tasks.task_created" | "tasks.task_status_changed", + when?: { field: string; in: (string | null)[] }, +): ProcessDefinition { + return { + subject: { recordType: event === "employees.added" ? "employee" : "task" }, + trigger: { + kind: "on_event", + event, + run: "in_same_transaction", + faultMode: "rollback_transaction", + ...(when && { when }), + }, + startableByAdmin: false, + steps: [ + { + key: "follow_up", + kind: "task", + title: "Follow up", + description: null, + assignee: { kind: "subject" }, + due: { amount: 0, unit: "calendar_days" }, + }, + ], + }; +} From 690c2d9452eed25113fee6385ee48c8b6e76d5fd Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 13:51:23 +0100 Subject: [PATCH 05/14] fix(processes): lock tasks before a reset deletes employees, so it waits for a task change --- packages/api/src/modules/index.ts | 2 + .../modules/processes/app/registrations.ts | 2 +- .../app/startRunsOnEventsAtScale.int.test.ts | 54 +++++++++++++++++++ .../api/src/modules/tasks/app/taskService.ts | 10 ++++ .../src/modules/tasks/db/taskRepository.ts | 6 +++ packages/api/test/support/timeOffService.ts | 1 + 6 files changed, 74 insertions(+), 1 deletion(-) diff --git a/packages/api/src/modules/index.ts b/packages/api/src/modules/index.ts index 467971105..b08de2ff6 100644 --- a/packages/api/src/modules/index.ts +++ b/packages/api/src/modules/index.ts @@ -63,6 +63,8 @@ export function createModules( await timeOff.lockPolicies(ctx, tx); // Then the processes: a step command locks its process before its run and task. await processes.lockProcesses(ctx, tx); + // Then the tasks: a task change can create tasks for employees before it commits. + await tasks.lockTasks(ctx, tx); // Employees next: their assignments, allowances and ledger lines point at the policies. await employeeSetup.deleteAllEmployees(ctx, tx); await timeOff.resetTimeOff(ctx, tx); diff --git a/packages/api/src/modules/processes/app/registrations.ts b/packages/api/src/modules/processes/app/registrations.ts index 2d9ac7859..b6a50488b 100644 --- a/packages/api/src/modules/processes/app/registrations.ts +++ b/packages/api/src/modules/processes/app/registrations.ts @@ -20,7 +20,7 @@ declare module "../../../core/jobs" { declare module "../../../core/events" { interface DomainEvents { - /** Runs of the process started with the change whose events triggered them, now committed. */ + /** Runs started in the transaction whose events triggered them; after-commit handlers get it only once it commits. */ "processes.runs_started_on_event": { processKey: string; runIds: string[]; eventIds: string[] }; } } diff --git a/packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts b/packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts index 94c8bf0c8..76458cd08 100644 --- a/packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts +++ b/packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts @@ -3,9 +3,13 @@ import { sql } from "drizzle-orm"; import { ADMIN, calendarDate, + deleteAllEmployees, eventProcess, expect, + expectBlockedBy, processService, + signedIn, + taskService, test, transactWithProcesses, } from "#test/support"; @@ -65,4 +69,54 @@ test( }, ); +test( + "a reset waits for a task change whose event starts a run, and removes that run", + { timeout: 5_000 }, + async ({ orm, seed, transact }) => { + const employee = await seed.employee(); + const task = await seed.task({ + assigneeEmployeeId: employee.id, + askedByEmployeeId: employee.id, + subjectEmployeeId: employee.id, + }); + const { key } = await seed.process({ + key: "follow_up", + definition: eventProcess("tasks.task_status_changed", { field: "to", in: ["cancelled"] }), + }); + const ctx = signedIn(orm, { employeeId: employee.id, profiles: ["EMPLOYEE"] }); + const locked = Promise.withResolvers(); + const release = Promise.withResolvers(); + const changing = transactWithProcesses(orm, { today })(async (tx) => { + await taskService(orm, { transact: (work) => work(tx) }).cancel( + ctx, + task.id, + "No longer needed", + ); + const { rows } = await ormOf(tx).execute<{ pid: number }>( + sql`select pg_backend_pid() as pid`, + ); + locked.resolve(rows[0]!.pid); + await release.promise; + }); + await Promise.race([locked.promise, changing]); + const resetting = transact((tx) => deleteAllEmployees(orm)(ADMIN, tx)); + const results = Promise.allSettled([changing, resetting]); + + try { + await expectBlockedBy(orm, await locked.promise); + } finally { + release.resolve(); + await results; + } + + expect(await results).toMatchObject([{ status: "fulfilled" }, { status: "fulfilled" }]); + expect(await processService(orm).listRuns(ADMIN, key)).toEqual([]); + expect(await taskService(orm).listMyTasks(ctx)).toEqual({ + needsYou: [], + waitingOnOthers: [], + resolvedRecently: [], + }); + }, +); + const today = () => calendarDate("2026-12-03"); diff --git a/packages/api/src/modules/tasks/app/taskService.ts b/packages/api/src/modules/tasks/app/taskService.ts index 33dd8b353..23ec9b2c1 100644 --- a/packages/api/src/modules/tasks/app/taskService.ts +++ b/packages/api/src/modules/tasks/app/taskService.ts @@ -63,6 +63,11 @@ export interface TaskService { reopen(ctx: SecurityContext, taskId: string): Promise; /** The person who asked withdraws an open task, saying why. */ cancel(ctx: SecurityContext, taskId: string, reason: string): Promise; + /** + * Trusted, for resets: locks every task until the transaction ends, before the employees whose + * deletion takes their tasks, so a reset waits for a task change instead of deadlocking with it. + */ + lockTasks(ctx: SecurityContext, tx: Tx): Promise; /** * Back to a fresh workspace when setup or the demo goes back to its start, after the employees, * whose deletion took their tasks: forgets the tasks' history. @@ -184,6 +189,11 @@ export function createTaskService({ return cancelled; }, + async lockTasks(ctx, tx) { + ctx.requireSetupAccess("MANAGE_WORKSPACE"); + await taskRepository(tx).lockAll(); + }, + async resetTasks(ctx, tx) { ctx.requireSetupAccess("MANAGE_WORKSPACE"); await history.purge(tx, "task"); diff --git a/packages/api/src/modules/tasks/db/taskRepository.ts b/packages/api/src/modules/tasks/db/taskRepository.ts index c6934e1bd..e2029c373 100644 --- a/packages/api/src/modules/tasks/db/taskRepository.ts +++ b/packages/api/src/modules/tasks/db/taskRepository.ts @@ -17,6 +17,8 @@ export interface TaskRepository { lockById(id: string): Promise; /** The tasks among these ids, locked in id order until the transaction ends. */ lockByIds(ids: readonly string[]): Promise; + /** Every task, locked in id order until the transaction ends. */ + lockAll(): Promise; /** Writes the task's status and the columns that follow it. */ saveStatus(task: Task): Promise; /** @@ -52,6 +54,10 @@ export function taskRepository(db: Orm | Tx): TaskRepository { return rows.map(toTask); }, + async lockAll() { + await orm.select({ id: tasks.id }).from(tasks).orderBy(tasks.id).for("update"); + }, + async lockByIds(ids) { if (ids.length === 0) return []; const rows = await orm diff --git a/packages/api/test/support/timeOffService.ts b/packages/api/test/support/timeOffService.ts index b5e7c1564..9e4749602 100644 --- a/packages/api/test/support/timeOffService.ts +++ b/packages/api/test/support/timeOffService.ts @@ -257,6 +257,7 @@ export function deleteAllEmployees(orm: Orm) { return async (ctx: SecurityContext, tx: Tx) => { await timeOff.lockPolicies(ctx, tx); await processes.lockProcesses(ctx, tx); + await tasks.lockTasks(ctx, tx); await employeeSetup.deleteAllEmployees(ctx, tx); await timeOff.resetTimeOff(ctx, tx); await processes.resetProcesses(ctx, tx); From 8da12c697ed0b7d415efb6bef42d27837c34505b Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 14:00:29 +0100 Subject: [PATCH 06/14] test(processes): seed event processes with seedProcess, keeping the test seed under its line limit --- .../app/startRunsOnEvents.int.test.ts | 41 ++++++++++--------- .../app/startRunsOnEventsAtScale.int.test.ts | 7 ++-- packages/api/test/support/index.ts | 2 +- packages/api/test/support/testSeed.ts | 4 -- 4 files changed, 26 insertions(+), 28 deletions(-) diff --git a/packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts b/packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts index c24b4853a..2ca81ea4a 100644 --- a/packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts +++ b/packages/api/src/modules/processes/app/startRunsOnEvents.int.test.ts @@ -6,6 +6,7 @@ import { employeeServices, expect, processService, + seedProcess, signedIn, taskService, test, @@ -24,7 +25,7 @@ test("adding a person commits exactly one process run and a task for their manag const manager = await seed.employee(); const definition = eventProcess("employees.added"); definition.steps[0]!.assignee = { kind: "subjects_manager" }; - const { key } = await seed.process({ key: "welcome", definition }); + const { key } = await seedProcess(orm, { key: "welcome", definition }); const transact = transactWithProcesses(orm, { today }); const employee = await transact(async (tx) => { @@ -63,7 +64,7 @@ test("adding a person commits exactly one process run and a task for their manag test("a failed transaction leaves no event-started run", async ({ orm, seed }) => { const employee = await seed.employee(); - const { key } = await seed.process({ + const { key } = await seedProcess(orm, { key: "welcome", definition: eventProcess("employees.added"), }); @@ -84,7 +85,7 @@ test("a failed transaction leaves no event-started run", async ({ orm, seed }) = test("one event for three people starts three runs", async ({ orm, seed }) => { const people = await seed.employees(3); const ids = people.map(({ id }) => id); - const { key } = await seed.process({ + const { key } = await seedProcess(orm, { key: "welcome", definition: eventProcess("employees.added"), }); @@ -107,7 +108,7 @@ test("one event for three people starts three runs", async ({ orm, seed }) => { test("three events in one transaction start three runs", async ({ orm, seed }) => { const people = await seed.employees(3); const ids = people.map(({ id }) => id); - const { key } = await seed.process({ + const { key } = await seedProcess(orm, { key: "welcome", definition: eventProcess("employees.added"), }); @@ -123,13 +124,10 @@ test("three events in one transaction start three runs", async ({ orm, seed }) = expect(new Set(runs.map(({ triggerEvent }) => triggerEvent!.id)).size).toBe(3); }); -test("a run that cannot calculate its due date rolls back adding the person", async ({ - orm, - seed, -}) => { +test("a run that cannot calculate its due date rolls back adding the person", async ({ orm }) => { const definition = eventProcess("employees.added"); definition.steps[0]!.due = { amount: 1, unit: "business_days" }; - const { key } = await seed.process({ key: "welcome", definition }); + const { key } = await seedProcess(orm, { key: "welcome", definition }); const transact = transactWithProcesses(orm, { today: () => calendarDate("2031-06-02") }); const employees = employeeServices(orm, { transact }); @@ -145,8 +143,8 @@ test("a run that cannot calculate its due date rolls back adding the person", as expect(await processService(orm).listRuns(ADMIN, key)).toEqual([]); }); -test("an off process starts nothing when its event happens", async ({ orm, seed }) => { - const { key } = await seed.process({ +test("an off process starts nothing when its event happens", async ({ orm }) => { + const { key } = await seedProcess(orm, { key: "welcome", definition: eventProcess("employees.added"), turnedOn: false, @@ -162,9 +160,9 @@ test("an off process starts nothing when its event happens", async ({ orm, seed expect(await processService(orm).listRuns(ADMIN, key)).toEqual([]); }); -test("an unmatched event condition starts nothing", async ({ orm, seed }) => { +test("an unmatched event condition starts nothing", async ({ orm }) => { const definition = eventProcess("employees.added", { field: "source", in: ["import"] }); - const { key } = await seed.process({ key: "welcome", definition }); + const { key } = await seedProcess(orm, { key: "welcome", definition }); const transact = transactWithProcesses(orm, { today }); await employeeServices(orm, { transact }).createEmployee(ADMIN, { @@ -191,7 +189,7 @@ test("marking a task done starts a process carrying the person who acted", async field: "to", in: ["done", "cancelled"], }); - const { key } = await seed.process({ key: "follow_up", definition }); + const { key } = await seedProcess(orm, { key: "follow_up", definition }); const transact = transactWithProcesses(orm, { today }); const ctx = signedIn(orm, { employeeId: assignee.id, profiles: ["EMPLOYEE"] }); @@ -222,7 +220,7 @@ test("marking a task waiting does not start a done-or-cancelled process", async field: "to", in: ["done", "cancelled"], }); - const { key } = await seed.process({ key: "follow_up", definition }); + const { key } = await seedProcess(orm, { key: "follow_up", definition }); const transact = transactWithProcesses(orm, { today }); const tasks = taskService(orm, { transact }); const ctx = signedIn(orm, { employeeId: assignee.id, profiles: ["EMPLOYEE"] }); @@ -233,10 +231,13 @@ test("marking a task waiting does not start a done-or-cancelled process", async expect(await processService(orm).listRuns(ADMIN, key)).toEqual([]); }); -test("a run's tasks start another process in the same transaction", async ({ orm, seed }) => { - const first = await seed.process({ key: "welcome", definition: eventProcess("employees.added") }); +test("a run's tasks start another process in the same transaction", async ({ orm }) => { + const first = await seedProcess(orm, { + key: "welcome", + definition: eventProcess("employees.added"), + }); const definition = eventProcess("tasks.task_created", { field: "processKey", in: [first.key] }); - const second = await seed.process({ key: "follow_up", definition }); + const second = await seedProcess(orm, { key: "follow_up", definition }); const transact = transactWithProcesses(orm, { today }); const added = await employeeServices(orm, { transact }).createEmployee(ADMIN, { @@ -268,7 +269,7 @@ test("two matching events start two runs when every step is skipped", async ({ o const employee = await seed.employee(); const definition = eventProcess("employees.added"); definition.steps[0]!.assignee = { kind: "subjects_manager" }; - const { key } = await seed.process({ key: "welcome", definition }); + const { key } = await seedProcess(orm, { key: "welcome", definition }); const transact = transactWithProcesses(orm, { today }); await transact((tx) => { @@ -287,7 +288,7 @@ test("a matching event during a run in progress is skipped and the save commits" seed, }) => { const employee = await seed.employee(); - const { key } = await seed.process({ + const { key } = await seedProcess(orm, { key: "welcome", definition: eventProcess("employees.added"), }); diff --git a/packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts b/packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts index 76458cd08..dded330b4 100644 --- a/packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts +++ b/packages/api/src/modules/processes/app/startRunsOnEventsAtScale.int.test.ts @@ -8,6 +8,7 @@ import { expect, expectBlockedBy, processService, + seedProcess, signedIn, taskService, test, @@ -24,7 +25,7 @@ test("one event for more people than one insert can hold starts a run for each", const people = await seed.employees(5_000); const definition = eventProcess("employees.added"); definition.steps.push({ ...definition.steps[0]!, key: "second_follow_up" }); - const { key } = await seed.process({ key: "welcome", definition }); + const { key } = await seedProcess(orm, { key: "welcome", definition }); const transact = transactWithProcesses(orm, { today }); await transact((tx) => { @@ -40,7 +41,7 @@ test( { timeout: 5_000 }, async ({ orm, seed, transact }) => { const employee = await seed.employee(); - const process = await seed.process({ + const process = await seedProcess(orm, { key: "welcome", definition: eventProcess("employees.added"), }); @@ -79,7 +80,7 @@ test( askedByEmployeeId: employee.id, subjectEmployeeId: employee.id, }); - const { key } = await seed.process({ + const { key } = await seedProcess(orm, { key: "follow_up", definition: eventProcess("tasks.task_status_changed", { field: "to", in: ["cancelled"] }), }); diff --git a/packages/api/test/support/index.ts b/packages/api/test/support/index.ts index 7bfcf1698..d7773f3a9 100644 --- a/packages/api/test/support/index.ts +++ b/packages/api/test/support/index.ts @@ -79,7 +79,7 @@ export { } from "./services"; export type { EmployeeServices } from "./services"; export { processService, transactWithProcesses } from "./processService"; -export { eventProcess } from "./seedProcess"; +export { eventProcess, seedProcess } from "./seedProcess"; export { domainEvents } from "./domainEvents"; export { employeeSetup, diff --git a/packages/api/test/support/testSeed.ts b/packages/api/test/support/testSeed.ts index 820645818..83adaf07d 100644 --- a/packages/api/test/support/testSeed.ts +++ b/packages/api/test/support/testSeed.ts @@ -20,7 +20,6 @@ import { defaultTermsId, insertTerms, leaveTerms, moveToTerms } from "./seedTerm import { seedUser } from "./seedUser"; import { seedAgentConnection } from "./seedAgentConnection"; import { seedTask } from "./seedTask"; -import { seedProcess } from "./seedProcess"; import { testTransactor } from "./testTransactor"; type SeedEmployee = Partial< @@ -187,9 +186,6 @@ export class TestSeed { agentConnection = (options: Parameters[1]) => seedAgentConnection(this.orm, options); - /** A process with its first version published and active. */ - process = (options: Parameters[1]) => seedProcess(this.orm, options); - /** The id of the seeded "England and Wales" terms, which every new employee joins. */ defaultTermsId = () => defaultTermsId(this.orm); From 40d0ebad22d9a71a9981021735989f36b9817a23 Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 14:13:42 +0100 Subject: [PATCH 07/14] refactor(processes): export only what other folders use --- packages/api/src/core/db/index.ts | 4 ++-- packages/api/src/core/db/transaction.int.test.ts | 3 ++- packages/api/src/modules/processes/db/index.ts | 7 +------ packages/api/src/modules/processes/domain/index.ts | 2 -- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/api/src/core/db/index.ts b/packages/api/src/core/db/index.ts index b1cc9140a..7b75873d0 100644 --- a/packages/api/src/core/db/index.ts +++ b/packages/api/src/core/db/index.ts @@ -6,11 +6,11 @@ export { anyOf } from "./anyOf"; export { jsonb } from "./jsonb"; export { afterKeyset, keysetOrder, nullsLast, orderByKeyset } from "./keyset"; export { createBeforeCommitBus } from "./beforeCommitBus"; -export { MAX_BEFORE_COMMIT_ROUNDS, ormOf, transactor } from "./transaction"; +export { ormOf, transactor } from "./transaction"; export { isForeignKeyViolation, isUniqueViolation } from "./uniqueViolation"; // Public types. -export type { BeforeCommitBus, BeforeCommitHandler } from "./beforeCommitBus"; +export type { BeforeCommitHandler } from "./beforeCommitBus"; export type { Database, Orm, OrmTransaction } from "./database"; export type { KeysetOrder } from "./keyset"; export type { Transact, Tx } from "./transaction"; diff --git a/packages/api/src/core/db/transaction.int.test.ts b/packages/api/src/core/db/transaction.int.test.ts index ecbed6bd2..15bb626cb 100644 --- a/packages/api/src/core/db/transaction.int.test.ts +++ b/packages/api/src/core/db/transaction.int.test.ts @@ -2,7 +2,8 @@ import { EXAMPLE_PAYLOAD, expect, SYSTEM, test, instances, setupSteps } from "#t import { createEventBus, type PublishedEvent } from "../events"; import { createLogger } from "../logger"; -import { createBeforeCommitBus, MAX_BEFORE_COMMIT_ROUNDS, transactor } from "."; +import { createBeforeCommitBus, transactor } from "."; +import { MAX_BEFORE_COMMIT_ROUNDS } from "./transaction"; test("dispatches queued events after the write commits", async ({ orm, seed }) => { await seed.owner(); diff --git a/packages/api/src/modules/processes/db/index.ts b/packages/api/src/modules/processes/db/index.ts index 51b2fa362..ca42d14d2 100644 --- a/packages/api/src/modules/processes/db/index.ts +++ b/packages/api/src/modules/processes/db/index.ts @@ -1,6 +1 @@ -export { - type NewProcessRow, - type NewRunRow, - processRepository, - type StoredProcess, -} from "./processRepository"; +export { type NewRunRow, processRepository, type StoredProcess } from "./processRepository"; diff --git a/packages/api/src/modules/processes/domain/index.ts b/packages/api/src/modules/processes/domain/index.ts index 0ce6ed2ff..ee029cd2e 100644 --- a/packages/api/src/modules/processes/domain/index.ts +++ b/packages/api/src/modules/processes/domain/index.ts @@ -5,12 +5,10 @@ export { type EventStart, isTriggeringEvent, matchEventStarts, - TRIGGERING_EVENTS, type TriggerEvent, } from "./eventTrigger"; export { type DayOffset, - type EventTrigger, parseProcessDefinition, PROCESS_DEFINITION_SCHEMA_VERSION, type ProcessDefinition, From 510d0ac83200b0d9bdf01efdcd96b2e9f4b5c140 Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 13:59:20 +0100 Subject: [PATCH 08/14] feat: show your agent connecting and disconnecting at once on Your agent --- .../app/agentConnectionService.int.test.ts | 69 ++++++++++++++++- .../agentAccess/app/agentConnectionService.ts | 9 ++- .../core/agentAccess/domain/registrations.ts | 10 +++ packages/api/src/core/live/index.ts | 5 ++ packages/api/src/core/live/liveNudges.ts | 74 ++++++++++++++++++ packages/api/src/core/server.ts | 6 ++ packages/api/test/e2e/liveNudges.test.ts | 75 +++++++++++++++++++ .../test/support/agentConnectionService.ts | 9 ++- packages/api/test/support/domainEvents.ts | 6 +- packages/api/test/support/index.ts | 1 + packages/api/test/support/sseMessages.ts | 42 +++++++++++ packages/web/src/features/mcpAgents/index.ts | 1 + .../mcpAgents/useLiveAgentConnection.ts | 32 ++++++++ packages/web/src/shell/AppShell.tsx | 2 + 14 files changed, 334 insertions(+), 7 deletions(-) create mode 100644 packages/api/src/core/agentAccess/domain/registrations.ts create mode 100644 packages/api/src/core/live/index.ts create mode 100644 packages/api/src/core/live/liveNudges.ts create mode 100644 packages/api/test/e2e/liveNudges.test.ts create mode 100644 packages/api/test/support/sseMessages.ts create mode 100644 packages/web/src/features/mcpAgents/useLiveAgentConnection.ts diff --git a/packages/api/src/core/agentAccess/app/agentConnectionService.int.test.ts b/packages/api/src/core/agentAccess/app/agentConnectionService.int.test.ts index 508b1d96d..26ce381b1 100644 --- a/packages/api/src/core/agentAccess/app/agentConnectionService.int.test.ts +++ b/packages/api/src/core/agentAccess/app/agentConnectionService.int.test.ts @@ -1,6 +1,73 @@ import { afterEach, beforeEach, vi } from "vitest"; -import { ADMIN, agentConnectionService, expect, OWNER, SYSTEM, test } from "#test/support"; +import { + ADMIN, + agentConnectionService, + domainEvents, + expect, + OWNER, + SYSTEM, + test, +} from "#test/support"; + +test("disconnect publishes one connection change for the caller", async ({ orm, seed }) => { + const userId = OWNER.user!.userId; + await seed.agentConnection({ userId }); + await seed.agentConnection({ userId }); + const { transact, events } = domainEvents(orm); + + await agentConnectionService(orm, { transact }).disconnectMyAgent(OWNER); + + expect(events).toEqual([{ type: "agentAccess.connection_changed", payload: { userId } }]); +}); + +test("disconnect without a connection publishes nothing", async ({ orm }) => { + const { transact, events } = domainEvents(orm); + + await agentConnectionService(orm, { transact }).disconnectMyAgent(OWNER); + + expect(events).toEqual([]); +}); + +test("allowing an agent publishes a connection change for that person", async ({ orm, seed }) => { + const userId = OWNER.user!.userId; + const { clientId } = await seed.agentConnection({ userId }); + const { transact, events } = domainEvents(orm); + + await agentConnectionService(orm, { transact }).keepOnlyConnection(SYSTEM, { userId, clientId }); + + expect(events).toEqual([{ type: "agentAccess.connection_changed", payload: { userId } }]); +}); + +test("a password reset publishes one connection change for that person", async ({ orm, seed }) => { + const userId = OWNER.user!.userId; + await seed.agentConnection({ userId }); + await seed.agentConnection({ userId }); + const { transact, events } = domainEvents(orm); + + await agentConnectionService(orm, { transact }).endConnections(SYSTEM, { + userId, + reason: "ended_by_password_reset", + }); + + expect(events).toEqual([{ type: "agentAccess.connection_changed", payload: { userId } }]); +}); + +test("using a connection unused for 31 days publishes a connection change", async ({ + orm, + seed, +}) => { + const userId = OWNER.user!.userId; + const { clientId } = await seed.agentConnection({ + userId, + connectedAt: new Date("2026-10-12T12:00:00Z"), + }); + const { transact, events } = domainEvents(orm); + + await agentConnectionService(orm, { transact }).useAgentConnection(SYSTEM, { userId, clientId }); + + expect(events).toEqual([{ type: "agentAccess.connection_changed", payload: { userId } }]); +}); test("disconnect ends the caller's connection and refuses its next use", async ({ orm, seed }) => { const connectedAt = new Date("2026-11-11T12:00:00Z"); diff --git a/packages/api/src/core/agentAccess/app/agentConnectionService.ts b/packages/api/src/core/agentAccess/app/agentConnectionService.ts index 75da66f3a..9fb7f10fa 100644 --- a/packages/api/src/core/agentAccess/app/agentConnectionService.ts +++ b/packages/api/src/core/agentAccess/app/agentConnectionService.ts @@ -67,6 +67,7 @@ export function createAgentConnectionService({ if (!(await connections.findConnectionOf(userId, clientId))) { return { ended: [], allowed: null }; } + tx.publish("agentAccess.connection_changed", { userId }); return { ended: await connections.endConnections(userId, clientId, { reason: "replaced_by_another_agent", @@ -141,7 +142,9 @@ async function endAllConnections( const ended = await transact(async (tx) => { const connections = agentConnectionRepository(tx); await connections.lockPerson(userId); - return connections.endConnections(userId, null, { reason, endedAt: new Date() }); + const ended = await connections.endConnections(userId, null, { reason, endedAt: new Date() }); + if (ended.length > 0) tx.publish("agentAccess.connection_changed", { userId }); + return ended; }); if (ended.length > 0) { logger.info( @@ -160,10 +163,12 @@ async function endUnusedConnection( const ended = await transact(async (tx) => { const connections = agentConnectionRepository(tx); await connections.lockPerson(userId); - return connections.endConnection(userId, clientId, { + const ended = await connections.endConnection(userId, clientId, { reason: "ended_after_30_days_unused", endedAt: new Date(), }); + if (ended) tx.publish("agentAccess.connection_changed", { userId }); + return ended; }); if (ended) { logger.info( diff --git a/packages/api/src/core/agentAccess/domain/registrations.ts b/packages/api/src/core/agentAccess/domain/registrations.ts new file mode 100644 index 000000000..350133b15 --- /dev/null +++ b/packages/api/src/core/agentAccess/domain/registrations.ts @@ -0,0 +1,10 @@ +// What agent access adds to core's shared registries. + +declare module "../../events" { + interface DomainEvents { + /** A person's agent connection was allowed or ended; their open pages read it again. */ + "agentAccess.connection_changed": { userId: string }; + } +} + +export {}; diff --git a/packages/api/src/core/live/index.ts b/packages/api/src/core/live/index.ts new file mode 100644 index 000000000..45e1d3bc7 --- /dev/null +++ b/packages/api/src/core/live/index.ts @@ -0,0 +1,5 @@ +// Public values. +export { createLiveNudges } from "./liveNudges"; + +// Public types. +export type { LiveNudges } from "./liveNudges"; diff --git a/packages/api/src/core/live/liveNudges.ts b/packages/api/src/core/live/liveNudges.ts new file mode 100644 index 000000000..b4ddfe143 --- /dev/null +++ b/packages/api/src/core/live/liveNudges.ts @@ -0,0 +1,74 @@ +import express from "express"; + +import { sendApplicationError } from "../applicationError"; +import type { EventHandlers } from "../events"; +import { requireSignedIn, type SecurityResolver } from "../security"; + +/** Often enough that proxies never close a quiet stream. */ +const HEARTBEAT_MS = 25_000; +/** The only message a stream sends: read the agent connection again. Never data. */ +const AGENT_CONNECTION_CHANGED = `data: ${JSON.stringify({ type: "agentConnection.changed" })}\n\n`; + +/** + * `GET /api/live`: a server-sent events stream per signed-in browser tab that tells the page when + * to read again, so the page shows a change at once without the stream carrying any data. Streams + * live in this process; one API process serves an instance. + */ +export interface LiveNudges { + router: express.Router; + /** Nudges a person's open streams after their agent connection changes. */ + events: EventHandlers; + /** Ends every open stream, so the server can stop without waiting for them. */ + close(): void; +} + +export function createLiveNudges(resolveSecurity: SecurityResolver): LiveNudges { + const streams = new Map>(); + + const router = express.Router().get("/live", async (request, response) => { + let userId: string; + try { + const { security, renewedCookies } = await resolveSecurity(request.headers); + const user = requireSignedIn(security).user; + if (!user) { + response.sendStatus(401); + return; + } + userId = user.userId; + if (renewedCookies.length > 0) response.set("Set-Cookie", renewedCookies); + } catch (error) { + sendApplicationError(error, response); + return; + } + response + .status(200) + .set({ + "Content-Type": "text/event-stream", + "Cache-Control": "no-store", + // Tells a buffering proxy to pass each message on at once. + "X-Accel-Buffering": "no", + }) + .flushHeaders(); + const own = streams.get(userId) ?? new Set(); + streams.set(userId, own.add(response)); + const heartbeat = setInterval(() => response.write(": heartbeat\n\n"), HEARTBEAT_MS); + response.on("close", () => { + clearInterval(heartbeat); + own.delete(response); + if (own.size === 0) streams.delete(userId); + }); + }); + + return { + router, + events: { + "agentAccess.connection_changed": ({ userId }) => { + for (const stream of streams.get(userId) ?? []) stream.write(AGENT_CONNECTION_CHANGED); + return Promise.resolve(); + }, + }, + close() { + for (const stream of [...streams.values()].flatMap((own) => [...own])) stream.end(); + }, + }; +} diff --git a/packages/api/src/core/server.ts b/packages/api/src/core/server.ts index a97102d5b..83aad4e48 100644 --- a/packages/api/src/core/server.ts +++ b/packages/api/src/core/server.ts @@ -20,6 +20,7 @@ import { createEventBus } from "./events"; import { type GraphqlApi, startGraphqlApi } from "./graphql"; import { healthRouter } from "./health"; import { createJobRunner } from "./jobs"; +import { createLiveNudges, type LiveNudges } from "./live"; import { refuseBearerAuth, resolveSecurityContext, selfHostedServer } from "./instance"; import type { Logger } from "./logger"; import { managedServer } from "./managedSignIn"; @@ -71,6 +72,7 @@ export async function startServer( let resolveAgentSecurity: AgentSecurityResolver; const agentConnections = createAgentConnectionService({ orm: database.orm, transact, logger }); let graphql: GraphqlApi; + let live: LiveNudges; try { const { manifests, setupDemo, addApprovedPeople, employeeDirectory, passwordResetEmail } = createModules({ @@ -106,6 +108,8 @@ export async function startServer( const auth = deployment.auth; resolveSecurity = (headers) => resolveSecurityContext(auth, database.orm, employeeDirectory, headers); + live = createLiveNudges(resolveSecurity); + eventBus.subscribe(live.events); resolveAgentSecurity = agentSecurityResolver( auth, database.orm, @@ -143,6 +147,7 @@ export async function startServer( ); } } + app.use("/api", live.router); app.all("/api/auth/{*path}", authHandler(deployment.auth)); app.use(graphql.router); if (webApp) { @@ -166,6 +171,7 @@ export async function startServer( stop: () => { stopping ??= (async () => { stopped.abort(); + live.close(); // Drains in-flight requests, then closes the HTTP server. await graphql.stop(); await jobRunner.stop(); diff --git a/packages/api/test/e2e/liveNudges.test.ts b/packages/api/test/e2e/liveNudges.test.ts new file mode 100644 index 000000000..06f62efe9 --- /dev/null +++ b/packages/api/test/e2e/liveNudges.test.ts @@ -0,0 +1,75 @@ +import { + connectAgent, + expect, + registerAgentClient, + sseMessages, + test, + TEST_OWNER, +} from "#test/support"; + +test("connecting an agent and disconnecting each send the person's stream one nudge", async ({ + env, +}) => { + const { browser } = await env.signInAsOwner(); + const clientId = await registerAgentClient(env, "Claude Code"); + const response = await browser.get("/api/live"); + const messages = sseMessages(response); + try { + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toMatch(/^text\/event-stream/); + expect(response.headers.get("cache-control")).toBe("no-store"); + + await connectAgent(env, browser, { clientId }); + + expect(await messages.next()).toBe('{"type":"agentConnection.changed"}'); + + const disconnected = await browser.graphql( + "mutation { mcpAgents { disconnect { endedAgentConnection { reason } } } }", + ); + + expect(await disconnected.json()).toEqual({ + data: { + mcpAgents: { disconnect: { endedAgentConnection: { reason: "DISCONNECTED_BY_PERSON" } } }, + }, + }); + expect(await messages.next()).toBe('{"type":"agentConnection.changed"}'); + expect(await messages.next(100)).toBeNull(); + } finally { + await messages.cancel(); + } +}); + +test("refuses a live stream without a session", async ({ env }) => { + await env.signInAsOwner(); + + const response = await env.browser().get("/api/live"); + + try { + expect(response.status).toBe(401); + } finally { + await response.body?.cancel(); + } +}); + +test("a person's stream receives no nudge when another person's connection changes", async ({ + env, +}) => { + const { browser: owner } = await env.signInAsOwner(); + const credentials = { email: "another@example.com", password: TEST_OWNER.password }; + await env.seed.user(credentials); + const { browser } = await env.signIn(credentials); + const clientId = await registerAgentClient(env, "Claude Code"); + const response = await browser.get("/api/live"); + const messages = sseMessages(response); + try { + expect(response.status).toBe(200); + + await connectAgent(env, owner, { clientId }); + await connectAgent(env, browser, { clientId }); + + expect(await messages.next()).toBe('{"type":"agentConnection.changed"}'); + expect(await messages.next(100)).toBeNull(); + } finally { + await messages.cancel(); + } +}); diff --git a/packages/api/test/support/agentConnectionService.ts b/packages/api/test/support/agentConnectionService.ts index 7b0360a23..c68f4f0df 100644 --- a/packages/api/test/support/agentConnectionService.ts +++ b/packages/api/test/support/agentConnectionService.ts @@ -1,12 +1,15 @@ import { createAgentConnectionService } from "../../src/core/agentAccess"; -import type { Orm } from "../../src/core/db"; +import type { Orm, Transact } from "../../src/core/db"; import { createLogger } from "../../src/core/logger"; import { testTransactor } from "./testTransactor"; /** People's agent connections on `orm`, with a silent logger. */ -export const agentConnectionService = (orm: Orm) => +export const agentConnectionService = ( + orm: Orm, + { transact = testTransactor(orm) }: { transact?: Transact } = {}, +) => createAgentConnectionService({ orm, - transact: testTransactor(orm), + transact, logger: createLogger("silent"), }); diff --git a/packages/api/test/support/domainEvents.ts b/packages/api/test/support/domainEvents.ts index 2b9c9bc10..32802d1a4 100644 --- a/packages/api/test/support/domainEvents.ts +++ b/packages/api/test/support/domainEvents.ts @@ -2,11 +2,15 @@ import { type Orm, transactor } from "../../src/core/db"; import { createEventBus, type DomainEvent } from "../../src/core/events"; import { createLogger } from "../../src/core/logger"; -/** Task events delivered after the transaction commits. */ +/** Task and agent connection events delivered after the transaction commits. */ export function domainEvents(orm: Orm) { const events: DomainEvent[] = []; const eventBus = createEventBus(createLogger("silent")); eventBus.subscribe({ + "agentAccess.connection_changed": (payload) => { + events.push({ type: "agentAccess.connection_changed", payload }); + return Promise.resolve(); + }, "tasks.task_created": (payload) => { events.push({ type: "tasks.task_created", payload }); return Promise.resolve(); diff --git a/packages/api/test/support/index.ts b/packages/api/test/support/index.ts index d7773f3a9..906128dd0 100644 --- a/packages/api/test/support/index.ts +++ b/packages/api/test/support/index.ts @@ -81,6 +81,7 @@ export type { EmployeeServices } from "./services"; export { processService, transactWithProcesses } from "./processService"; export { eventProcess, seedProcess } from "./seedProcess"; export { domainEvents } from "./domainEvents"; +export { sseMessages } from "./sseMessages"; export { employeeSetup, fileStorage, diff --git a/packages/api/test/support/sseMessages.ts b/packages/api/test/support/sseMessages.ts new file mode 100644 index 000000000..7c5367ee7 --- /dev/null +++ b/packages/api/test/support/sseMessages.ts @@ -0,0 +1,42 @@ +import assert from "node:assert/strict"; + +/** Reads SSE data, preserving messages across chunks; a timeout returns null and cancels the stream. */ +export function sseMessages(response: Response) { + assert(response.body, "The SSE response has no body"); + const reader: ReadableStreamDefaultReader = response.body.getReader(); + const decoder = new TextDecoder(); + let buffer = ""; + + return { + cancel: () => reader.cancel(), + async next(timeoutMs = 2_000): Promise { + let timer: ReturnType; + const timeout = new Promise((resolve) => { + timer = setTimeout(() => resolve(null), timeoutMs); + }); + try { + for (;;) { + const boundary = /\r?\n\r?\n/.exec(buffer); + if (boundary) { + const lines = buffer.slice(0, boundary.index).split(/\r?\n/); + buffer = buffer.slice(boundary.index + boundary[0].length); + const data = lines.filter((line) => line.startsWith("data:")); + if (data.length > 0) { + return data.map((line) => line.slice(5).replace(/^ /, "")).join("\n"); + } + continue; + } + const chunk = await Promise.race([reader.read(), timeout]); + if (chunk === null) { + await reader.cancel(); + return null; + } + assert(!chunk.done, "The SSE stream ended before the next message"); + buffer += decoder.decode(chunk.value, { stream: true }); + } + } finally { + clearTimeout(timer!); + } + }, + }; +} diff --git a/packages/web/src/features/mcpAgents/index.ts b/packages/web/src/features/mcpAgents/index.ts index bdfed1c6e..fd2d568a2 100644 --- a/packages/web/src/features/mcpAgents/index.ts +++ b/packages/web/src/features/mcpAgents/index.ts @@ -1,2 +1,3 @@ export { AllowAgentPage } from "./AllowAgentPage"; export { YourAgentPage } from "./YourAgentPage"; +export { useLiveAgentConnection } from "./useLiveAgentConnection"; diff --git a/packages/web/src/features/mcpAgents/useLiveAgentConnection.ts b/packages/web/src/features/mcpAgents/useLiveAgentConnection.ts new file mode 100644 index 000000000..1352632c7 --- /dev/null +++ b/packages/web/src/features/mcpAgents/useLiveAgentConnection.ts @@ -0,0 +1,32 @@ +import { useApolloClient } from "@apollo/client/react"; +import { useEffect } from "react"; + +/** The open queries that show the person's agent connection. */ +const AGENT_CONNECTION_QUERIES = new Set(["YourAgent", "LeftNavAgentConnection"]); + +/** + * Keeps the person's agent connection current while they are signed in: the server nudges this + * tab when their agent connects or a connection ends, and the open queries that show it read again. + * After a dropped stream the browser reconnects by itself, and the queries read again then too. + */ +export function useLiveAgentConnection(): void { + const apolloClient = useApolloClient(); + useEffect(() => { + const refetch = () => + void apolloClient.refetchQueries({ + include: "active", + onQueryUpdated: (query) => AGENT_CONNECTION_QUERIES.has(query.queryName ?? ""), + }); + const stream = new EventSource("/api/live"); + let dropped = false; + stream.onmessage = refetch; + stream.onerror = () => { + dropped = true; + }; + stream.onopen = () => { + if (dropped) refetch(); + dropped = false; + }; + return () => stream.close(); + }, [apolloClient]); +} diff --git a/packages/web/src/shell/AppShell.tsx b/packages/web/src/shell/AppShell.tsx index 782aa121a..a7dfeb0cb 100644 --- a/packages/web/src/shell/AppShell.tsx +++ b/packages/web/src/shell/AppShell.tsx @@ -2,6 +2,7 @@ import { useRef, type ReactNode } from "react"; import { Toaster } from "@/components/ui/sonner"; import { DemoBanner } from "@/features/demo"; +import { useLiveAgentConnection } from "@/features/mcpAgents"; import { cn } from "@/lib/class-names"; @@ -17,6 +18,7 @@ export function AppShell({ children }: { children: ReactNode }) { const main = useRef(null); const inSettings = useInSettings(); useFocusHeadingOnNavigation(main); + useLiveAgentConnection(); return ( <> From 7b968cb969eccf201d5904e2544b052199e9aa9c Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 13:59:31 +0100 Subject: [PATCH 09/14] docs: live updates of the agent connection --- docs/architecture.md | 1 + docs/qa/connect-your-agent.md | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/architecture.md b/docs/architecture.md index 1a630ca33..4e69a811d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -64,6 +64,7 @@ For example, a domain rule decides whether an import source may retry. The appli - **Setup access stays separate.** The owner's coding agent during setup keeps its one-time code and dedicated HTTP routes; its session is limited to setup and never becomes a person's agent connection. - **Transport and authority.** Stateless Streamable HTTP at `/api/mcp`, authorized by OAuth 2.1 with PKCE. Better Auth's OAuth provider is the authorization server at `/api/auth`, for public clients registered dynamically or through a client metadata document. Tokens are opaque and checked in the database on every call, so an ended connection is refused at once; each call resolves the person's security context afresh, as for their browser, and a token must name this instance's MCP address as its resource. Two scopes, read and write, are intersected with the person's permissions; each tool declares the scope it needs. Resources and prompts remain outside the initial scope. - **One connection per person.** Allowing an agent ends the person's other agents' tokens and consents; a password reset ends them all. A token is issued within the browser session that allowed it, so the server detaches tokens from expired browser sessions before issuing new ones, and the connection outlives the browser's sign-in. +- **Live updates.** A signed-in browser keeps one server-sent events stream open at `/api/live`. It carries only a nudge that the person's agent connection changed, never data; the page reads it again through GraphQL, so record scope stays in one place. Nudges follow the commit through the in-process event bus, and streams live in the one API process an instance runs. ## 4. Data access diff --git a/docs/qa/connect-your-agent.md b/docs/qa/connect-your-agent.md index 606a9bd00..3ca1a8055 100644 --- a/docs/qa/connect-your-agent.md +++ b/docs/qa/connect-your-agent.md @@ -33,7 +33,9 @@ A manual check of connecting a person's own agent over MCP against a development - [ ] "Your agent" is in the menu for every signed-in person, and its page shows the address the browser is on plus `/api/mcp`, with Copy. - [ ] On a self-hosted instance, a line under the address says agents work only at the address they connected to, with a link to set `SELF_HOSTED_PUBLIC_URL`. - [ ] The Claude, ChatGPT and "Claude Code or Codex" tabs show their steps; each command's Copy copies it with this address. -- [ ] Following the Claude Code tab connects it; coming back to the window shows the agent, "Connected …· Last active …" and a dot on the menu item. +- [ ] Following the Claude Code tab connects it; while the agent's Allow is chosen, Your agent (left open, not focused) turns to the agent, "Connected …· Last active …" and a dot on the menu item within a second. +- [ ] With Your agent open in two tabs, Disconnect in one turns the other to "You disconnected Claude Code." within a second; so does a password reset in another browser. +- [ ] After the network drops and comes back (DevTools offline, then online), a Disconnect made meanwhile shows without a reload. - [ ] Within an hour of connecting, the card opens with "Claude Code is connected. Nice."; × hides it. Each "Try asking" prompt's Copy copies it, and pasting it into a new session of the agent gets an answer (Codex loads Broccoli's tools only in a session started after login). - [ ] "Disconnect", then "Disconnect" in the confirm, shows "You disconnected Claude Code."; the agent's next call is refused. - [ ] After a password reset by email and signing in again, the page says the agent was disconnected when the password was reset. From d064e5159ab11a68d3044805a41b2c3fc76d90d6 Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 14:04:00 +0100 Subject: [PATCH 10/14] fix: open the live stream with a comment so a proxy passes its headers at once --- packages/api/src/core/live/liveNudges.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/api/src/core/live/liveNudges.ts b/packages/api/src/core/live/liveNudges.ts index b4ddfe143..6930127cf 100644 --- a/packages/api/src/core/live/liveNudges.ts +++ b/packages/api/src/core/live/liveNudges.ts @@ -48,7 +48,8 @@ export function createLiveNudges(resolveSecurity: SecurityResolver): LiveNudges // Tells a buffering proxy to pass each message on at once. "X-Accel-Buffering": "no", }) - .flushHeaders(); + // A proxy may hold the headers back until the first bytes, so the stream opens with a comment. + .write(": open\n\n"); const own = streams.get(userId) ?? new Set(); streams.set(userId, own.add(response)); const heartbeat = setInterval(() => response.write(": heartbeat\n\n"), HEARTBEAT_MS); From 7f34fd8e9544c45ed1e9824a2384427321c58fa8 Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 14:10:28 +0100 Subject: [PATCH 11/14] refactor: keep the live stream and its event with agent access --- packages/api/importRules.json | 1 + .../src/core/agentAccess/{domain => app}/registrations.ts | 0 packages/api/src/core/agentAccess/http/index.ts | 2 ++ .../api/src/core/{live => agentAccess/http}/liveNudges.ts | 6 +++--- packages/api/src/core/agentAccess/index.ts | 3 ++- packages/api/src/core/live/index.ts | 5 ----- packages/api/src/core/server.ts | 3 ++- 7 files changed, 10 insertions(+), 10 deletions(-) rename packages/api/src/core/agentAccess/{domain => app}/registrations.ts (100%) rename packages/api/src/core/{live => agentAccess/http}/liveNudges.ts (93%) delete mode 100644 packages/api/src/core/live/index.ts diff --git a/packages/api/importRules.json b/packages/api/importRules.json index 9447198b0..f07cd7e0d 100644 --- a/packages/api/importRules.json +++ b/packages/api/importRules.json @@ -111,6 +111,7 @@ "src/core/applicationError", "src/core/auth", "src/core/db", + "src/core/events", "src/core/instance", "src/core/logger", "src/core/security" diff --git a/packages/api/src/core/agentAccess/domain/registrations.ts b/packages/api/src/core/agentAccess/app/registrations.ts similarity index 100% rename from packages/api/src/core/agentAccess/domain/registrations.ts rename to packages/api/src/core/agentAccess/app/registrations.ts diff --git a/packages/api/src/core/agentAccess/http/index.ts b/packages/api/src/core/agentAccess/http/index.ts index b8abca24d..7471c08e3 100644 --- a/packages/api/src/core/agentAccess/http/index.ts +++ b/packages/api/src/core/agentAccess/http/index.ts @@ -1 +1,3 @@ export { agentDiscoveryRouter } from "./agentDiscoveryRouter"; +export { createLiveNudges } from "./liveNudges"; +export type { LiveNudges } from "./liveNudges"; diff --git a/packages/api/src/core/live/liveNudges.ts b/packages/api/src/core/agentAccess/http/liveNudges.ts similarity index 93% rename from packages/api/src/core/live/liveNudges.ts rename to packages/api/src/core/agentAccess/http/liveNudges.ts index 6930127cf..b64358d85 100644 --- a/packages/api/src/core/live/liveNudges.ts +++ b/packages/api/src/core/agentAccess/http/liveNudges.ts @@ -1,8 +1,8 @@ import express from "express"; -import { sendApplicationError } from "../applicationError"; -import type { EventHandlers } from "../events"; -import { requireSignedIn, type SecurityResolver } from "../security"; +import { sendApplicationError } from "../../applicationError"; +import type { EventHandlers } from "../../events"; +import { requireSignedIn, type SecurityResolver } from "../../security"; /** Often enough that proxies never close a quiet stream. */ const HEARTBEAT_MS = 25_000; diff --git a/packages/api/src/core/agentAccess/index.ts b/packages/api/src/core/agentAccess/index.ts index 95077c3fb..f551f2124 100644 --- a/packages/api/src/core/agentAccess/index.ts +++ b/packages/api/src/core/agentAccess/index.ts @@ -2,10 +2,11 @@ export { createAgentConnectionService } from "./app"; export { agentAccessPlugins, agentSecurityResolver } from "./betterAuth"; export { agentResourceMetadataOf } from "./domain"; -export { agentDiscoveryRouter } from "./http"; +export { agentDiscoveryRouter, createLiveNudges } from "./http"; // Public types. export type { AgentConnectionService } from "./app"; export type { AgentSecurityResolver } from "./betterAuth"; export type { StoredEndedAgentConnection } from "./db"; export type { AgentScope } from "./domain"; +export type { LiveNudges } from "./http"; diff --git a/packages/api/src/core/live/index.ts b/packages/api/src/core/live/index.ts deleted file mode 100644 index 45e1d3bc7..000000000 --- a/packages/api/src/core/live/index.ts +++ /dev/null @@ -1,5 +0,0 @@ -// Public values. -export { createLiveNudges } from "./liveNudges"; - -// Public types. -export type { LiveNudges } from "./liveNudges"; diff --git a/packages/api/src/core/server.ts b/packages/api/src/core/server.ts index 83aad4e48..10016ab18 100644 --- a/packages/api/src/core/server.ts +++ b/packages/api/src/core/server.ts @@ -12,6 +12,8 @@ import { agentSecurityResolver, type AgentSecurityResolver, createAgentConnectionService, + createLiveNudges, + type LiveNudges, } from "./agentAccess"; import { authHandler, type DeploymentServer } from "./auth"; import { connectDatabase, createBeforeCommitBus, type Database, transactor } from "./db"; @@ -20,7 +22,6 @@ import { createEventBus } from "./events"; import { type GraphqlApi, startGraphqlApi } from "./graphql"; import { healthRouter } from "./health"; import { createJobRunner } from "./jobs"; -import { createLiveNudges, type LiveNudges } from "./live"; import { refuseBearerAuth, resolveSecurityContext, selfHostedServer } from "./instance"; import type { Logger } from "./logger"; import { managedServer } from "./managedSignIn"; From 759f6104b1b378b00cda6c2b51b69c751c8fbb9f Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 14:15:29 +0100 Subject: [PATCH 12/14] chore: let the shell keep the agent connection live --- packages/web/importRules.json | 1 + packages/web/src/features/mcpAgents/useLiveAgentConnection.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/web/importRules.json b/packages/web/importRules.json index 8bb1f7c76..a929bd8a1 100644 --- a/packages/web/importRules.json +++ b/packages/web/importRules.json @@ -22,6 +22,7 @@ "src/features/auth", "src/features/demo", "src/features/employees", + "src/features/mcpAgents", "src/features/setup", "src/features/tasks", "src/i18n", diff --git a/packages/web/src/features/mcpAgents/useLiveAgentConnection.ts b/packages/web/src/features/mcpAgents/useLiveAgentConnection.ts index 1352632c7..d77952bb4 100644 --- a/packages/web/src/features/mcpAgents/useLiveAgentConnection.ts +++ b/packages/web/src/features/mcpAgents/useLiveAgentConnection.ts @@ -1,3 +1,4 @@ +// eslint-disable-next-line no-restricted-imports -- A nudge refetches open queries, which the cache alone cannot. import { useApolloClient } from "@apollo/client/react"; import { useEffect } from "react"; From eaf50c69418375ac9a42e159b345d75653f2b33e Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 14:23:00 +0100 Subject: [PATCH 13/14] fix: never keep a stream for a closed tab or a stopping server, and read again whenever it opens --- packages/api/src/core/agentAccess/http/liveNudges.ts | 7 +++++++ packages/web/importRules.json | 1 - .../mcpAgents/YourAgentPage/WaitingForAgent.tsx | 2 +- packages/web/src/features/mcpAgents/index.ts | 1 - packages/web/src/i18n/en/mcpAgents.ts | 2 +- packages/web/src/shell/AppShell.tsx | 2 +- .../mcpAgents => shell}/useLiveAgentConnection.ts | 12 +++--------- 7 files changed, 13 insertions(+), 14 deletions(-) rename packages/web/src/{features/mcpAgents => shell}/useLiveAgentConnection.ts (78%) diff --git a/packages/api/src/core/agentAccess/http/liveNudges.ts b/packages/api/src/core/agentAccess/http/liveNudges.ts index b64358d85..d439c7fbc 100644 --- a/packages/api/src/core/agentAccess/http/liveNudges.ts +++ b/packages/api/src/core/agentAccess/http/liveNudges.ts @@ -24,6 +24,7 @@ export interface LiveNudges { export function createLiveNudges(resolveSecurity: SecurityResolver): LiveNudges { const streams = new Map>(); + let closed = false; const router = express.Router().get("/live", async (request, response) => { let userId: string; @@ -40,6 +41,11 @@ export function createLiveNudges(resolveSecurity: SecurityResolver): LiveNudges sendApplicationError(error, response); return; } + // The tab may have gone, or the server begun stopping, while the session was read. + if (closed || response.destroyed) { + response.end(); + return; + } response .status(200) .set({ @@ -69,6 +75,7 @@ export function createLiveNudges(resolveSecurity: SecurityResolver): LiveNudges }, }, close() { + closed = true; for (const stream of [...streams.values()].flatMap((own) => [...own])) stream.end(); }, }; diff --git a/packages/web/importRules.json b/packages/web/importRules.json index a929bd8a1..8bb1f7c76 100644 --- a/packages/web/importRules.json +++ b/packages/web/importRules.json @@ -22,7 +22,6 @@ "src/features/auth", "src/features/demo", "src/features/employees", - "src/features/mcpAgents", "src/features/setup", "src/features/tasks", "src/i18n", diff --git a/packages/web/src/features/mcpAgents/YourAgentPage/WaitingForAgent.tsx b/packages/web/src/features/mcpAgents/YourAgentPage/WaitingForAgent.tsx index 3373b0654..e11e7fc29 100644 --- a/packages/web/src/features/mcpAgents/YourAgentPage/WaitingForAgent.tsx +++ b/packages/web/src/features/mcpAgents/YourAgentPage/WaitingForAgent.tsx @@ -2,7 +2,7 @@ import { t } from "@/i18n"; import "./waitingForAgent.css"; -/** Shown until the person's agent connects; the page checks again when they come back to it. */ +/** Shown until the person's agent connects, which the page shows the moment it happens. */ export function WaitingForAgent() { return (

diff --git a/packages/web/src/features/mcpAgents/index.ts b/packages/web/src/features/mcpAgents/index.ts index fd2d568a2..bdfed1c6e 100644 --- a/packages/web/src/features/mcpAgents/index.ts +++ b/packages/web/src/features/mcpAgents/index.ts @@ -1,3 +1,2 @@ export { AllowAgentPage } from "./AllowAgentPage"; export { YourAgentPage } from "./YourAgentPage"; -export { useLiveAgentConnection } from "./useLiveAgentConnection"; diff --git a/packages/web/src/i18n/en/mcpAgents.ts b/packages/web/src/i18n/en/mcpAgents.ts index 2c234c998..9e38f14de 100644 --- a/packages/web/src/i18n/en/mcpAgents.ts +++ b/packages/web/src/i18n/en/mcpAgents.ts @@ -51,7 +51,7 @@ export const mcpAgentsEn = { "Then run codex mcp login broccoli, choose Allow, and start a new Codex session.", "mcpAgents.cli.copyCommand": "Copy command", "mcpAgents.cli.other": "Other clients: add the address as a remote HTTP server with OAuth.", - "mcpAgents.waiting": "Waiting for your agent…", + "mcpAgents.waiting": "Waiting for your agent… This page updates the moment you choose Allow.", "mcpAgents.connected.badge": "Connected", "mcpAgents.connected.since": "Acts as you, {name} · Connected {date} · Last active {lastActive}", "mcpAgents.welcome.title": "{client} is connected. Nice.", diff --git a/packages/web/src/shell/AppShell.tsx b/packages/web/src/shell/AppShell.tsx index a7dfeb0cb..26769d816 100644 --- a/packages/web/src/shell/AppShell.tsx +++ b/packages/web/src/shell/AppShell.tsx @@ -2,13 +2,13 @@ import { useRef, type ReactNode } from "react"; import { Toaster } from "@/components/ui/sonner"; import { DemoBanner } from "@/features/demo"; -import { useLiveAgentConnection } from "@/features/mcpAgents"; import { cn } from "@/lib/class-names"; import { LeftNav } from "./LeftNav"; import { useFocusHeadingOnNavigation } from "./useFocusHeadingOnNavigation"; import { useInSettings } from "./useInSettings"; +import { useLiveAgentConnection } from "./useLiveAgentConnection"; /** * The window-sized app frame: navigation above or beside the Canvas, a card that diff --git a/packages/web/src/features/mcpAgents/useLiveAgentConnection.ts b/packages/web/src/shell/useLiveAgentConnection.ts similarity index 78% rename from packages/web/src/features/mcpAgents/useLiveAgentConnection.ts rename to packages/web/src/shell/useLiveAgentConnection.ts index d77952bb4..205a0b64f 100644 --- a/packages/web/src/features/mcpAgents/useLiveAgentConnection.ts +++ b/packages/web/src/shell/useLiveAgentConnection.ts @@ -8,7 +8,8 @@ const AGENT_CONNECTION_QUERIES = new Set(["YourAgent", "LeftNavAgentConnection"] /** * Keeps the person's agent connection current while they are signed in: the server nudges this * tab when their agent connects or a connection ends, and the open queries that show it read again. - * After a dropped stream the browser reconnects by itself, and the queries read again then too. + * They read again each time the stream opens too, for a change made while it was not yet or no + * longer open; the browser reopens a dropped stream by itself. */ export function useLiveAgentConnection(): void { const apolloClient = useApolloClient(); @@ -19,15 +20,8 @@ export function useLiveAgentConnection(): void { onQueryUpdated: (query) => AGENT_CONNECTION_QUERIES.has(query.queryName ?? ""), }); const stream = new EventSource("/api/live"); - let dropped = false; stream.onmessage = refetch; - stream.onerror = () => { - dropped = true; - }; - stream.onopen = () => { - if (dropped) refetch(); - dropped = false; - }; + stream.onopen = refetch; return () => stream.close(); }, [apolloClient]); } From f935ff56b38f6bc4195c2fd3b81526ae1f98298a Mon Sep 17 00:00:00 2001 From: Artem Kireev Date: Thu, 12 Nov 2026 14:27:47 +0100 Subject: [PATCH 14/14] fix: read the agent connection afresh on a nudge, never reusing a read already in flight --- docs/qa/connect-your-agent.md | 3 ++- packages/web/src/shell/useLiveAgentConnection.ts | 10 +++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/qa/connect-your-agent.md b/docs/qa/connect-your-agent.md index 3ca1a8055..bbaedcb52 100644 --- a/docs/qa/connect-your-agent.md +++ b/docs/qa/connect-your-agent.md @@ -34,7 +34,8 @@ A manual check of connecting a person's own agent over MCP against a development - [ ] On a self-hosted instance, a line under the address says agents work only at the address they connected to, with a link to set `SELF_HOSTED_PUBLIC_URL`. - [ ] The Claude, ChatGPT and "Claude Code or Codex" tabs show their steps; each command's Copy copies it with this address. - [ ] Following the Claude Code tab connects it; while the agent's Allow is chosen, Your agent (left open, not focused) turns to the agent, "Connected …· Last active …" and a dot on the menu item within a second. -- [ ] With Your agent open in two tabs, Disconnect in one turns the other to "You disconnected Claude Code." within a second; so does a password reset in another browser. +- [ ] With Your agent open in two tabs, Disconnect in one turns the other to "You disconnected Claude Code." within a second. +- [ ] With Your agent open, a password reset in another browser turns it to "Claude Code was disconnected when your password was reset." within a second. - [ ] After the network drops and comes back (DevTools offline, then online), a Disconnect made meanwhile shows without a reload. - [ ] Within an hour of connecting, the card opens with "Claude Code is connected. Nice."; × hides it. Each "Try asking" prompt's Copy copies it, and pasting it into a new session of the agent gets an answer (Codex loads Broccoli's tools only in a session started after login). - [ ] "Disconnect", then "Disconnect" in the confirm, shows "You disconnected Claude Code."; the agent's next call is refused. diff --git a/packages/web/src/shell/useLiveAgentConnection.ts b/packages/web/src/shell/useLiveAgentConnection.ts index 205a0b64f..eef43057a 100644 --- a/packages/web/src/shell/useLiveAgentConnection.ts +++ b/packages/web/src/shell/useLiveAgentConnection.ts @@ -17,7 +17,15 @@ export function useLiveAgentConnection(): void { const refetch = () => void apolloClient.refetchQueries({ include: "active", - onQueryUpdated: (query) => AGENT_CONNECTION_QUERIES.has(query.queryName ?? ""), + onQueryUpdated: (query) => + AGENT_CONNECTION_QUERIES.has(query.queryName ?? "") && + // A new read: one already in flight may have read the state before the change. + apolloClient.query({ + query: query.query, + variables: query.variables, + fetchPolicy: "network-only", + context: { queryDeduplication: false }, + }), }); const stream = new EventSource("/api/live"); stream.onmessage = refetch;