diff --git a/apps/hub/src/index.ts b/apps/hub/src/index.ts index f80c61d9a..9b2cc3716 100644 --- a/apps/hub/src/index.ts +++ b/apps/hub/src/index.ts @@ -89,7 +89,6 @@ import { isWorkbenchHostDefinitionName, listConnectedProviders, listDefaultInferencePreferences, - provisionSpaceWorkbench, startWorkflowCommand, sendWorkbenchMessage, settleConnectedService, @@ -2849,18 +2848,6 @@ export async function createHub(config: HubConfig) { return row !== undefined && row.workflowDefinitionId === definitionId; }, deliveryWorkbenchRequired: routineDeliveryWorkbenchRequired, - // A routine created with no `deliveryWorkbenchId` gets a brand-new - // space of its own, named after it, rather than a dead-end - // 400 — the same workbench-provisioning core `POST /chat/workbenches` - // uses (`@corbits/chat`'s `provisionSpaceWorkbench`), reused here - // instead of reimplemented. - deliverySpace: { - createDeliverySpace: (input) => - provisionSpaceWorkbench( - { tenancy: chatTenancy, store: chatStore }, - input, - ), - }, validateRoutineInput: routineInputValid, }), ); @@ -2938,23 +2925,6 @@ export async function createHub(config: HubConfig) { ); return hit?.workbenchId; }, - deliverySpace: { - createDeliverySpace: (input) => - provisionSpaceWorkbench( - { tenancy: chatTenancy, store: chatStore }, - input, - ), - }, - resolveTenantDomain: async (tenantId) => { - const row = await db.query.tenant.findFirst({ - where: eq(tenantTable.id, tenantId), - columns: { domain: true }, - }); - if (row === undefined) { - throw new Error(`No tenant "${tenantId}"`); - } - return row.domain; - }, validateRoutineInput: routineInputValid, }), ); diff --git a/packages/agent-directory/src/client.test.ts b/packages/agent-directory/src/client.test.ts index 01384dbb7..7b8c192c6 100644 --- a/packages/agent-directory/src/client.test.ts +++ b/packages/agent-directory/src/client.test.ts @@ -40,6 +40,20 @@ const workbenchHostInstance = { definitionName: "ins-0f1e2d3c4b5a69788796a5b4c3d2e1f0", }; +const dailyDigestDefinition = { + ...researcher, + id: "wfd_3", + name: "workbench-digest", + description: null, +}; + +const last30DaysResearchDefinition = { + ...researcher, + id: "wfd_4", + name: "last-30-days-research", + description: null, +}; + describe("purposeAgentDefinitions", () => { test("drops the chat anchor machinery's workbench-host definitions", () => { const result = purposeAgentDefinitions([ @@ -48,6 +62,15 @@ describe("purposeAgentDefinitions", () => { ]); expect(result).toEqual([researcher]); }); + + test("drops routine-only workflow catalog utilities (Daily digest, Last 30 days research) — they are non-conversational, seeded as routines, and belong on the Routines page, not the Agents list", () => { + const result = purposeAgentDefinitions([ + researcher, + dailyDigestDefinition, + last30DaysResearchDefinition, + ]); + expect(result).toEqual([researcher]); + }); }); const invitedAgentInstance = { diff --git a/packages/agent-directory/src/client.ts b/packages/agent-directory/src/client.ts index d90885691..44bbc1623 100644 --- a/packages/agent-directory/src/client.ts +++ b/packages/agent-directory/src/client.ts @@ -15,6 +15,7 @@ import { withDisplayNames, type WithDisplayName, } from "@corbits/chat/display-name"; +import { isConversationalWorkflowName } from "@corbits/workflow-catalog"; // `deriveDisplayName`/`humanizeSlug` (CL-6413) live in `@corbits/chat` // itself, not here: this package already depends on `@corbits/chat` (for @@ -42,16 +43,27 @@ export type UserFacingAgentInstance = { }; /** Every definition, minus the chat anchor machinery's workbench hosts — - * those are internal plumbing, never a user-facing agent. Definitions - * never need the run-id filter `purposeAgentInstances` below takes: - * definition rows aren't run rows, so a folded run's own id can never - * match here — an invited agent's real `definitionId` stays a - * legitimate, reusable template even though its *instance* is chat - * plumbing. */ + * those are internal plumbing, never a user-facing agent — and minus every + * non-conversational workflow-catalog utility (`workbench-digest`/"Daily + * digest", `last-30-days-research`/"Last 30 days research", …): those are + * mail-triggered automations a routine schedules, never something a person + * opens a chat with, so they belong on the Routines page only. The one + * distinguishing property is `isConversationalWorkflowName` — "can this be + * DMed" — the same test `listVisibleAgentDefinitions` already applies to the + * sidebar's DM list; `automatable` (schedulable as a routine) is orthogonal + * and never decides this. Definitions never need the run-id filter + * `purposeAgentInstances` below takes: definition rows aren't run rows, so a + * folded run's own id can never match here — an invited agent's real + * `definitionId` stays a legitimate, reusable template even though its + * *instance* is chat plumbing. */ export function purposeAgentDefinitions( definitions: readonly T[], ): readonly T[] { - return definitions.filter((d) => !isWorkbenchHostDefinitionName(d.name)); + return definitions.filter( + (d) => + !isWorkbenchHostDefinitionName(d.name) && + isConversationalWorkflowName(d.name), + ); } /** diff --git a/packages/hub-client/src/default-routines.ts b/packages/hub-client/src/default-routines.ts index 0fdefb221..e3951cdc8 100644 --- a/packages/hub-client/src/default-routines.ts +++ b/packages/hub-client/src/default-routines.ts @@ -207,6 +207,24 @@ export async function ensureDefaultRoutines( body, cookies, ); + if ( + created.status === 400 && + /deliveryWorkbenchId is required/.test(JSON.stringify(created.data)) + ) { + // This preset's definition needs somewhere to deliver to, and + // seeding names no workbench — a person hasn't picked one yet, and + // seeding must never invent one and name it after the routine + // (that's exactly the "Daily digest"/"New Workbench" pollution this + // preset-planting flow used to cause). The routine simply isn't + // planted until a member creates it by hand and picks a real + // destination. + log( + `routine "${preset.name}" skipped: its workflow needs a delivery ` + + `workbench and this preset names none — create it by hand and ` + + `pick one`, + ); + continue; + } if ( created.status === 400 && /is required/.test(JSON.stringify(created.data)) diff --git a/packages/hub-client/test/default-routines.test.ts b/packages/hub-client/test/default-routines.test.ts index aadcea4bf..485625c8a 100644 --- a/packages/hub-client/test/default-routines.test.ts +++ b/packages/hub-client/test/default-routines.test.ts @@ -170,6 +170,45 @@ describe("ensureDefaultRoutines", () => { ); }); + // The hub never auto-provisions a delivery workbench (that pollution — + // a workbench literally named "Daily digest" — is exactly what this + // fix removes): a delivery-required preset seeded with no workbench + // named gets this 400, and seeding must skip it honestly rather than + // failing the whole seed or fabricating a destination. + test("skips a delivery-required preset honestly when the hub 400s for lacking a named workbench", async () => { + const { lines, log } = collector(); + const handler: FakeHandler = (method, path) => { + if ( + method === "GET" && + path === `/api/tenants/${TENANT_ID}/workflows/definitions` + ) { + return deployedDefinitionsResponse(); + } + if (method === "GET" && path === `/api/tenants/${TENANT_ID}/routines`) { + return { status: 200, data: { items: [] } }; + } + if (method === "POST" && path === `/api/tenants/${TENANT_ID}/routines`) { + return { + status: 400, + data: { + error: { + code: "bad_request", + message: "deliveryWorkbenchId is required for this workflow", + }, + }, + }; + } + return undefined; + }; + + await ensureDefaultRoutines(fakeAPI(handler), [], TENANT_ID, log); + + const output = lines.join("\n"); + expect(output).toContain( + 'routine "Daily digest" skipped: its workflow needs a delivery workbench and this preset names none — create it by hand and pick one', + ); + }); + test("a re-seed matches every preset by presetKey — even renamed — and creates nothing twice", async () => { const { lines, log } = collector(); const handler: FakeHandler = (method, path) => { diff --git a/packages/routines/src/index.ts b/packages/routines/src/index.ts index 83f5d7a92..5b3477f3b 100644 --- a/packages/routines/src/index.ts +++ b/packages/routines/src/index.ts @@ -87,7 +87,6 @@ export type { RoutineLauncher, LaunchedRoutineRun, RunSummaryResolver, - DeliverySpacePort, } from "./routes"; export { createWorkflowRoutineRoutes } from "./workflow-routine-routes"; diff --git a/packages/routines/src/routes.ts b/packages/routines/src/routes.ts index d246421f6..b13ba2689 100644 --- a/packages/routines/src/routes.ts +++ b/packages/routines/src/routes.ts @@ -69,24 +69,6 @@ export interface RoutineLauncher { }): Promise; } -/** - * Optional port: provision a new delivery destination ("space") for a - * routine that didn't pick an existing workbench. Wired to `@corbits/chat` - * workbench creation at the hub. Returns the new workbench's id plus a - * `compensate` callback that undoes the provisioning — called if the - * routine row itself then fails to write, so a space is never left - * behind with nothing pointing at it. - */ -export interface DeliverySpacePort { - createDeliverySpace(input: { - tenantId: string; - tenantDomain: string; - creatorPrincipalId: string; - creatorUserId: string; - name: string; - }): Promise<{ workbenchId: string; compensate: () => Promise }>; -} - /** * Optional port: posts a plain-text notice into a workbench through the * host's existing chat platform — the same path a human's web-UI @@ -148,15 +130,6 @@ export type CreateRoutineRoutesDeps = { webhookTriggerId: string, definitionId: string, ) => Promise; - /** - * Provisions a brand-new space for a routine created with no - * `deliveryWorkbenchId`, named after the routine. When omitted, a - * routine whose workflow requires delivery and names no workbench - * still 400s exactly as before this port existed — a host that - * hasn't wired space creation yet keeps the prior, honest error - * instead of silently accepting a routine with nowhere to deliver. - */ - deliverySpace?: DeliverySpacePort | undefined; /** * Whether a routine on this definition must carry a `deliveryWorkbenchId` * — `false` for a workflow whose result never posts to a workbench at @@ -583,13 +556,12 @@ export function createRoutineRoutes( body.deliveryWorkbenchId === ""); // No workbench named and none needed: fall through with a null - // delivery workbench, unchanged from before this port existed. - // A workbench is named: use it as-is, unchanged. Only the third - // case — delivery required, nothing named — is new: a space - // named after the routine is auto-provisioned, the routine's - // default destination rather than a dead end. A host that - // hasn't wired `deliverySpace` yet keeps the prior 400. - if (needsDelivery && deps.deliverySpace === undefined) { + // delivery workbench. A workbench is named: use it as-is. Delivery + // required and nothing named: 400 — a routine's destination is + // always a workbench the person picked, never one invented and + // named after the routine (CL-6201's own auto-provisioning is gone; + // see this file's git history for the removed `deliverySpace` port). + if (needsDelivery) { return c.json( ErrorEnvelope( "bad_request", @@ -610,123 +582,53 @@ export function createRoutineRoutes( } } - // The space is provisioned before the routine row: `createRoutine` - // is a single insert and effectively never fails on its own, but - // if it somehow does, the freshly-made space is compensated - // (deleted) rather than left behind pointing at nothing — the - // same mint-then-compensate shape `@corbits/chat`'s own - // `POST /workbenches` uses for its tenant mint. - let provisionedSpace: - { workbenchId: string; compensate: () => Promise } | undefined; - if (needsDelivery && deps.deliverySpace !== undefined) { - provisionedSpace = await deps.deliverySpace.createDeliverySpace({ - tenantId: tenant.id, - tenantDomain: tenant.domain, - creatorPrincipalId: principal.id, - creatorUserId: principal.refId, - name: body.name, - }); - } - const deliveryWorkbenchId = - body.deliveryWorkbenchId ?? provisionedSpace?.workbenchId ?? null; + const deliveryWorkbenchId = body.deliveryWorkbenchId ?? null; let row: RoutineRow | undefined; let created = true; - try { - if (body.presetKey !== undefined) { - const result = await deps.store.createRoutineIfAbsent({ - tenantId: tenant.id, - name: body.name, - definitionId: body.definitionId, - trigger: body.trigger, - scope: body.scope, - input: body.input ?? {}, - // A seeded preset is born disabled: a schedule must never - // start firing (or announce itself) just because a bench - // was minted — the member enabling it is the announcement. - enabled: false, - deliveryWorkbenchId, - createdBy: principal.id, - presetKey: body.presetKey, - }); - if (result.outcome !== "tombstoned") { - row = result.row; - created = result.outcome === "created"; - } - } else { - row = await deps.store.createRoutine({ - tenantId: tenant.id, - name: body.name, - definitionId: body.definitionId, - trigger: body.trigger, - scope: body.scope, - input: body.input ?? {}, - deliveryWorkbenchId, - createdBy: principal.id, - }); - } - } catch (err) { - if (provisionedSpace !== undefined) { - log.error( - "Routine creation failed after provisioning space " + - "{workbenchId}; compensating the orphaned space", - { workbenchId: provisionedSpace.workbenchId, err }, - ); - try { - await provisionedSpace.compensate(); - } catch (compensationErr) { - log.error( - "Compensation failed for orphaned space {workbenchId} " + - "after routine creation failed; this space now has no " + - "routine pointing at it and requires manual cleanup", - { workbenchId: provisionedSpace.workbenchId, compensationErr }, - ); - } + if (body.presetKey !== undefined) { + const result = await deps.store.createRoutineIfAbsent({ + tenantId: tenant.id, + name: body.name, + definitionId: body.definitionId, + trigger: body.trigger, + scope: body.scope, + input: body.input ?? {}, + // A seeded preset is born disabled: a schedule must never + // start firing (or announce itself) just because a bench + // was minted — the member enabling it is the announcement. + enabled: false, + deliveryWorkbenchId, + createdBy: principal.id, + presetKey: body.presetKey, + }); + if (result.outcome !== "tombstoned") { + row = result.row; + created = result.outcome === "created"; } - throw err; + } else { + row = await deps.store.createRoutine({ + tenantId: tenant.id, + name: body.name, + definitionId: body.definitionId, + trigger: body.trigger, + scope: body.scope, + input: body.input ?? {}, + deliveryWorkbenchId, + createdBy: principal.id, + }); } // Lost the create-if-absent race (or this is a genuine re-seed): - // the preset row already exists. Any space this request just - // provisioned points at nothing real and is compensated (deleted) - // rather than left orphaned — the winning request's own row keeps - // whatever delivery workbench it resolved to. No fire, no notice: - // both already happened (or are about to happen) on the winning - // request. - // A member deleted this preset's routine: absence is their - // choice, so nothing is (re-)created — 204, and any space this - // request just provisioned is compensated like a lost race. + // the preset row already exists. A member deleted this preset's + // routine: absence is their choice, so nothing is (re-)created — + // 204. No fire, no notice: both already happened (or are about to + // happen) on the winning request. if (row === undefined) { - if (provisionedSpace !== undefined) { - try { - await provisionedSpace.compensate(); - } catch (compensationErr) { - log.error( - "Compensation failed for orphaned space {workbenchId} " + - "after refusing to resurrect a member-deleted preset " + - "routine; this space now has no routine pointing at it " + - "and requires manual cleanup", - { workbenchId: provisionedSpace.workbenchId, compensationErr }, - ); - } - } return c.body(null, 204); } if (!created) { - if (provisionedSpace !== undefined) { - try { - await provisionedSpace.compensate(); - } catch (compensationErr) { - log.error( - "Compensation failed for orphaned space {workbenchId} " + - "after losing a routine create-if-absent race; this " + - "space now has no routine pointing at it and requires " + - "manual cleanup", - { workbenchId: provisionedSpace.workbenchId, compensationErr }, - ); - } - } return c.json(routineView(row), 200); } diff --git a/packages/routines/src/workflow-routine-routes.test.ts b/packages/routines/src/workflow-routine-routes.test.ts index 8aaebb595..1226095d2 100644 --- a/packages/routines/src/workflow-routine-routes.test.ts +++ b/packages/routines/src/workflow-routine-routes.test.ts @@ -1,7 +1,9 @@ // Route-level tests for Myra's own workflow-run-authenticated routine // surface: authentication, tenant-scoped (not self-definition-scoped) -// create/list/update/run-now, and the deliverySpace auto-provision -// fallback. Mirrors `packages/agent-directory/test/workflow-capability-routes.test.ts`'s +// create/list/update/run-now, and delivery-target precedence (a named +// workbench, then the creating run's own workbench — never an +// auto-provisioned one). Mirrors +// `packages/agent-directory/test/workflow-capability-routes.test.ts`'s // auth-check shape and `./test/routes.test.ts`'s fakes for the tenant- // session routine route this surface parallels. import { expect, test } from "bun:test"; @@ -14,11 +16,7 @@ import { type WorkflowRunAuthenticator, } from "./workflow-routine-routes"; import { createInMemoryRoutineStore, type RoutineStore } from "./store"; -import type { - WorkbenchNoticePort, - DeliverySpacePort, - RoutineLauncher, -} from "./routes"; +import type { WorkbenchNoticePort, RoutineLauncher } from "./routes"; const TENANT_ID = "tnt_1"; const PRINCIPAL_ID = "prn_myra"; @@ -222,7 +220,7 @@ test("rejects an invalid trigger with a 400", async () => { expect(response.status).toBe(400); }); -test("400s when delivery is required, no workbench is named, and no deliverySpace is wired", async () => { +test("400s when delivery is required and no workbench is named — never auto-provisioned", async () => { const app = buildApp( buildDeps({ deliveryWorkbenchRequired: async () => true }), ); @@ -233,53 +231,9 @@ test("400s when delivery is required, no workbench is named, and no deliverySpac expect(response.status).toBe(400); }); -test("auto-provisions a delivery space via deliverySpace + resolveTenantDomain when none is named", async () => { - let seenInput: Record | undefined; - const deliverySpace: DeliverySpacePort = { - createDeliverySpace: (input) => { - seenInput = input; - return Promise.resolve({ - workbenchId: "ch_provisioned", - compensate: () => Promise.resolve(), - }); - }, - }; - const deps = buildDeps({ - deliveryWorkbenchRequired: async () => true, - deliverySpace, - resolveTenantDomain: async () => "acme.example", - }); - const app = buildApp(deps); - const { response, body } = await createRoutine(app, { - ...VALID_BODY, - deliveryWorkbenchId: undefined, - }); - expect(response.status).toBe(201); - expect(body["deliveryWorkbenchId"]).toBe("ch_provisioned"); - expect(seenInput).toEqual({ - tenantId: TENANT_ID, - tenantDomain: "acme.example", - creatorPrincipalId: PRINCIPAL_ID, - creatorUserId: RUN_ID, - name: "Morning digest", - }); -}); - -test("delivery defaults to the creating run's own workbench — no new space is provisioned", async () => { - let provisioned = false; - const deliverySpace: DeliverySpacePort = { - createDeliverySpace: () => { - provisioned = true; - return Promise.resolve({ - workbenchId: "ch_provisioned", - compensate: () => Promise.resolve(), - }); - }, - }; +test("delivery defaults to the creating run's own workbench when none is named", async () => { const deps = buildDeps({ deliveryWorkbenchRequired: async () => true, - deliverySpace, - resolveTenantDomain: async () => "acme.example", resolveRunWorkbench: async (tenantId, runId) => tenantId === TENANT_ID && runId === RUN_ID ? "ch_home" : undefined, }); @@ -290,30 +244,19 @@ test("delivery defaults to the creating run's own workbench — no new space is }); expect(response.status).toBe(201); expect(body["deliveryWorkbenchId"]).toBe("ch_home"); - expect(provisioned).toBe(false); }); -test("a run with no home workbench still auto-provisions a delivery space", async () => { - const deliverySpace: DeliverySpacePort = { - createDeliverySpace: () => - Promise.resolve({ - workbenchId: "ch_provisioned", - compensate: () => Promise.resolve(), - }), - }; +test("a run with no home workbench still 400s — no space is invented on its behalf", async () => { const deps = buildDeps({ deliveryWorkbenchRequired: async () => true, - deliverySpace, - resolveTenantDomain: async () => "acme.example", resolveRunWorkbench: async () => undefined, }); const app = buildApp(deps); - const { response, body } = await createRoutine(app, { + const { response } = await createRoutine(app, { ...VALID_BODY, deliveryWorkbenchId: undefined, }); - expect(response.status).toBe(201); - expect(body["deliveryWorkbenchId"]).toBe("ch_provisioned"); + expect(response.status).toBe(400); }); test("an explicit deliveryWorkbenchId always wins over the run's home workbench", async () => { diff --git a/packages/routines/src/workflow-routine-routes.ts b/packages/routines/src/workflow-routine-routes.ts index feda3c60d..b2a8efff8 100644 --- a/packages/routines/src/workflow-routine-routes.ts +++ b/packages/routines/src/workflow-routine-routes.ts @@ -29,7 +29,7 @@ import { Hono } from "hono"; import { type } from "arktype"; import { RoutineTrigger } from "./trigger"; -import type { RoutineRow, RoutineStore, UpdateRoutineInput } from "./store"; +import type { RoutineStore, UpdateRoutineInput } from "./store"; import { fireOnceTriggerIfNeeded, isDeliveryWorkbenchRequired, @@ -38,7 +38,6 @@ import { routineView, webhookTriggerValid, type WorkbenchNoticePort, - type DeliverySpacePort, type RoutineLauncher, } from "./routes"; @@ -115,33 +114,15 @@ export type CreateWorkflowRoutineRoutesDeps = { /** * Resolves the creating run's own workbench — the workbench the person * was talking in when they asked for the routine. A routine created - * with no `deliveryWorkbenchId` delivers there by default; a brand-new - * space (via `deliverySpace` below) is only ever minted for a run - * with no home workbench of its own. + * with no `deliveryWorkbenchId` delivers there by default; there is no + * further fallback — a routine's destination is always a workbench a + * person is actually in or explicitly names, never one invented and + * named after the routine. */ resolveRunWorkbench?: ( tenantId: string, runId: string, ) => Promise; - /** - * Same contract as `CreateRoutineRoutesDeps.deliverySpace`: provisions - * a brand-new space for a routine created with no `deliveryWorkbenchId` - * by a run that `resolveRunWorkbench` cannot place in a workbench, named - * after the routine. - */ - deliverySpace?: DeliverySpacePort | undefined; - /** - * Resolves the tenant's domain, needed only for the `deliverySpace` - * auto-provision fallback: `DeliverySpacePort.createDeliverySpace` - * requires `tenantDomain`, which a workflow run's authenticated scope - * never carries (`WorkflowRoutineRunScope` above deliberately matches - * `WorkflowCapabilityRunScope`'s minimal `{tenantId, principalId, - * runId}` shape). Omitted disables auto-provisioning even when - * `deliverySpace` is wired — a routine needing delivery with no - * workbench named still 400s, exactly like a host that never wired - * `deliverySpace` at all. - */ - resolveTenantDomain?: (tenantId: string) => Promise; /** Same contract as `CreateRoutineRoutesDeps.deliveryWorkbenchRequired`. */ deliveryWorkbenchRequired?: ( tenantId: string, @@ -290,8 +271,10 @@ export function createWorkflowRoutineRoutes( // Delivery target precedence: the workbench the caller named, then // the creating run's own workbench (a routine asked for inside a - // workbench reports back into that workbench), then a freshly - // provisioned space as the last resort. + // workbench reports back into that workbench). Nothing is ever + // auto-provisioned — a routine's destination is always a workbench a + // person picked or was actually in, never one invented and named + // after the routine. const namedWorkbenchId = body.deliveryWorkbenchId !== undefined && body.deliveryWorkbenchId !== "" ? body.deliveryWorkbenchId @@ -312,11 +295,7 @@ export function createWorkflowRoutineRoutes( namedWorkbenchId === undefined && homeWorkbenchId === undefined; - if ( - needsDelivery && - (deps.deliverySpace === undefined || - deps.resolveTenantDomain === undefined) - ) { + if (needsDelivery) { return c.json( ErrorEnvelope( "bad_request", @@ -337,59 +316,18 @@ export function createWorkflowRoutineRoutes( } } - // The space is provisioned before the routine row, and compensated - // (deleted) if the row then fails to write — the same mint-then- - // compensate shape `./routes.ts`'s own `POST /routines` uses. - let provisionedSpace: - { workbenchId: string; compensate: () => Promise } | undefined; - if ( - needsDelivery && - deps.deliverySpace !== undefined && - deps.resolveTenantDomain !== undefined - ) { - const tenantDomain = await deps.resolveTenantDomain(scope.tenantId); - provisionedSpace = await deps.deliverySpace.createDeliverySpace({ - tenantId: scope.tenantId, - tenantDomain, - creatorPrincipalId: scope.principalId, - // A workflow-run principal has no separate "user" id the way a - // human tenant-session principal's `refId` names the underlying - // user — its own run IS the acting identity, mirroring the - // `refId = runId` convention `vendor/intx/hub-api`'s own grant - // materialization uses for a workflow-kind principal. - creatorUserId: scope.runId, - name: body.name, - }); - } - const deliveryWorkbenchId = - namedWorkbenchId ?? - homeWorkbenchId ?? - provisionedSpace?.workbenchId ?? - null; - - let row: RoutineRow; - try { - row = await deps.store.createRoutine({ - tenantId: scope.tenantId, - name: body.name, - definitionId, - trigger: body.trigger, - scope: "bench", - input: body.input ?? {}, - deliveryWorkbenchId, - createdBy: scope.principalId, - }); - } catch (err) { - if (provisionedSpace !== undefined) { - try { - await provisionedSpace.compensate(); - } catch { - // Best-effort: an orphaned space now requires manual cleanup, - // same fallback `./routes.ts`'s own compensation failure takes. - } - } - throw err; - } + const deliveryWorkbenchId = namedWorkbenchId ?? homeWorkbenchId ?? null; + + const row = await deps.store.createRoutine({ + tenantId: scope.tenantId, + name: body.name, + definitionId, + trigger: body.trigger, + scope: "bench", + input: body.input ?? {}, + deliveryWorkbenchId, + createdBy: scope.principalId, + }); if (body.runOnceNow === true) { await launchAndCorrelate( diff --git a/packages/routines/test/routes.test.ts b/packages/routines/test/routes.test.ts index 54e10f71c..ef60a9879 100644 --- a/packages/routines/test/routes.test.ts +++ b/packages/routines/test/routes.test.ts @@ -712,137 +712,10 @@ describe("createRoutineRoutes", () => { }); }); - describe("deliverySpace", () => { - function fakeDeliverySpace( - overrides: { - readonly workbenchId?: string; - readonly onCreate?: () => void; - readonly onCompensate?: () => void; - readonly failCompensate?: boolean; - } = {}, - ) { - const workbenchId = overrides.workbenchId ?? "ch_new_space"; - let createCalls = 0; - let compensateCalls = 0; - return { - get createCalls() { - return createCalls; - }, - get compensateCalls() { - return compensateCalls; - }, - async createDeliverySpace(input: { - tenantId: string; - tenantDomain: string; - creatorPrincipalId: string; - creatorUserId: string; - name: string; - }) { - createCalls += 1; - overrides.onCreate?.(); - expect(input.tenantId).toBe(TENANT.id); - expect(input.tenantDomain).toBe(TENANT.domain); - return { - workbenchId, - compensate: async () => { - compensateCalls += 1; - overrides.onCompensate?.(); - if (overrides.failCompensate === true) { - throw new Error("compensation failed"); - } - }, - }; - }, - }; - } - - test("provisions a new space and binds it as deliveryWorkbenchId when none is named", async () => { - const deliverySpace = fakeDeliverySpace(); - const deps = buildDeps({ deliverySpace }); - const app = mountAs(createRoutineRoutes(deps), "user_1"); - const { deliveryWorkbenchId: _drop, ...withoutWorkbench } = VALID_BODY; - const { response, body } = await createRoutine(app, { - ...withoutWorkbench, - name: "Weekly digest", - }); - expect(response.status).toBe(201); - expect(body["deliveryWorkbenchId"]).toBe("ch_new_space"); - expect(deliverySpace.createCalls).toBe(1); - }); - - test("leaves an existing deliveryWorkbenchId untouched — the space port is never called", async () => { - const deliverySpace = fakeDeliverySpace(); - const deps = buildDeps({ deliverySpace }); - const app = mountAs(createRoutineRoutes(deps), "user_1"); - const { response, body } = await createRoutine(app, VALID_BODY); - expect(response.status).toBe(201); - expect(body["deliveryWorkbenchId"]).toBe(VALID_BODY.deliveryWorkbenchId); - expect(deliverySpace.createCalls).toBe(0); - }); - - test("compensates (deletes) the provisioned space when the routine row then fails to write", async () => { - const deliverySpace = fakeDeliverySpace(); - const store = createInMemoryRoutineStore(); - store.createRoutine = async () => { - throw new Error("db unavailable"); - }; - const deps = buildDeps({ store, deliverySpace }); - const app = mountAs(createRoutineRoutes(deps), "user_1"); - const { deliveryWorkbenchId: _drop, ...withoutWorkbench } = VALID_BODY; - const response = await app.request("/routines", { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ ...withoutWorkbench, name: "Doomed" }), - }); - expect(response.status).toBe(500); - expect(deliverySpace.createCalls).toBe(1); - expect(deliverySpace.compensateCalls).toBe(1); - }); - - test("a routine row write failure still propagates even if compensation itself fails", async () => { - const deliverySpace = fakeDeliverySpace({ failCompensate: true }); - const store = createInMemoryRoutineStore(); - store.createRoutine = async () => { - throw new Error("db unavailable"); - }; - const deps = buildDeps({ store, deliverySpace }); - const app = mountAs(createRoutineRoutes(deps), "user_1"); - const { deliveryWorkbenchId: _drop, ...withoutWorkbench } = VALID_BODY; - const response = await app.request("/routines", { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ ...withoutWorkbench, name: "Doomed" }), - }); - expect(response.status).toBe(500); - expect(deliverySpace.compensateCalls).toBe(1); - }); - }); - // CL-6375: a template-minted routine (e.g. a seeded default preset) // must never re-create itself or re-announce on a second seed call — // real create-if-absent, not a check-then-insert race. describe("presetKey (create-if-absent)", () => { - function fakeDeliverySpaceMinting() { - let createCalls = 0; - const compensatedWorkbenchIds: string[] = []; - return { - get createCalls() { - return createCalls; - }, - compensatedWorkbenchIds, - async createDeliverySpace() { - createCalls += 1; - const workbenchId = `ch_minted_${createCalls}`; - return { - workbenchId, - compensate: async () => { - compensatedWorkbenchIds.push(workbenchId); - }, - }; - }, - }; - } - test("a second create with the same presetKey returns 200 and reuses the first row, never a second one", async () => { const deps = buildDeps(); const app = mountAs(createRoutineRoutes(deps), "user_1"); @@ -908,49 +781,6 @@ describe("createRoutineRoutes", () => { expect(await deps.store.listRoutines(TENANT.id)).toHaveLength(0); }); - test("a member-deleted preset re-create compensates the space it provisioned", async () => { - const deliverySpace = fakeDeliverySpaceMinting(); - const deps = buildDeps({ deliverySpace }); - const app = mountAs(createRoutineRoutes(deps), "user_1"); - const { deliveryWorkbenchId: _drop, ...withoutWorkbench } = VALID_BODY; - const body = { ...withoutWorkbench, presetKey: "workbench-digest" }; - - const first = await createRoutine(app, body); - const deleted = await app.request(`/routines/${first.body["id"]}`, { - method: "DELETE", - }); - expect(deleted.status).toBe(204); - - const again = await app.request("/routines", { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify(body), - }); - expect(again.status).toBe(204); - expect(deliverySpace.compensatedWorkbenchIds).toEqual(["ch_minted_2"]); - }); - - test("a losing create-if-absent request compensates (deletes) the space it just provisioned", async () => { - const deliverySpace = fakeDeliverySpaceMinting(); - const deps = buildDeps({ deliverySpace }); - const app = mountAs(createRoutineRoutes(deps), "user_1"); - const { deliveryWorkbenchId: _drop, ...withoutWorkbench } = VALID_BODY; - const body = { ...withoutWorkbench, presetKey: "workbench-digest" }; - - const first = await createRoutine(app, body); - const second = await createRoutine(app, body); - - expect(first.response.status).toBe(201); - expect(second.response.status).toBe(200); - // The winner's own space stays bound; the loser's freshly-minted - // space is the one compensated away. - expect(second.body["deliveryWorkbenchId"]).toBe( - first.body["deliveryWorkbenchId"], - ); - expect(deliverySpace.createCalls).toBe(2); - expect(deliverySpace.compensatedWorkbenchIds).toEqual(["ch_minted_2"]); - }); - test("a plain create (no presetKey) is unaffected — two same-named routines are both created", async () => { const deps = buildDeps(); const app = mountAs(createRoutineRoutes(deps), "user_1");