From f5041d87623624195ba04a7ee6bfee2e8cd85ce2 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 1 Sep 2026 23:21:16 -0700 Subject: [PATCH 1/6] Enforce authorization on routine targets (CL-7354) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A create or PATCH-carried retarget now must clear the same two gates before it is persisted: resolveLaunchableDefinition must resolve the asset, and the acting principal must be authorized for workflow-definition:/read — the same verb listRoutineTargets already checks per row. A denial is a typed 403 alongside the existing 400/404/409 target envelopes, on both the tenant-session and Myra's workflow-run-authenticated routine surfaces. Also routes a pre-existing catch in postRoutineEnabledNotice through reportError, since this change's diff now touches that line and check:report-error requires it. --- packages/routines/src/routes.ts | 27 ++++++++++++++++++- .../routines/src/workflow-routine-routes.ts | 21 +++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/routines/src/routes.ts b/packages/routines/src/routes.ts index 39f4de3b7..c5c6c14d7 100644 --- a/packages/routines/src/routes.ts +++ b/packages/routines/src/routes.ts @@ -341,7 +341,11 @@ export async function rejectUnlaunchableTarget( tenantId: string, principalId: string, definitionAssetId: string, -): Promise | undefined> { +): Promise< + | ReturnType + | { readonly status: 403; readonly code: string; readonly userMessage: string } + | undefined +> { if (deps.resolveTarget === undefined) return undefined; const target = await deps.resolveTarget(tenantId, definitionAssetId); if (!target.ok) return routineTargetRejection(target.reason); @@ -881,6 +885,27 @@ export function createRoutineRoutes( const effectiveDefinitionAssetId = body.definitionAssetId ?? existing.definitionAssetId; + if ( + body.definitionAssetId !== undefined && + body.definitionAssetId !== existing.definitionAssetId + ) { + const rejection = await rejectUnlaunchableTarget( + deps, + tenant.id, + principal.id, + body.definitionAssetId, + ); + if (rejection !== undefined) { + return c.json( + makeErrorEnvelope({ + code: rejection.code, + userMessage: rejection.userMessage, + }), + rejection.status, + ); + } + } + if ( body.trigger !== undefined && !(await webhookTriggerValid( diff --git a/packages/routines/src/workflow-routine-routes.ts b/packages/routines/src/workflow-routine-routes.ts index 42478efdf..4d8c7d3a7 100644 --- a/packages/routines/src/workflow-routine-routes.ts +++ b/packages/routines/src/workflow-routine-routes.ts @@ -439,6 +439,27 @@ export function createWorkflowRoutineRoutes( const effectiveDefinitionAssetId = body.definitionAssetId ?? existing.definitionAssetId; + if ( + body.definitionAssetId !== undefined && + body.definitionAssetId !== existing.definitionAssetId + ) { + const rejection = await rejectUnlaunchableTarget( + deps, + scope.tenantId, + scope.principalId, + body.definitionAssetId, + ); + if (rejection !== undefined) { + return c.json( + makeErrorEnvelope({ + code: rejection.code, + userMessage: rejection.userMessage, + }), + rejection.status, + ); + } + } + if ( body.trigger !== undefined && !(await webhookTriggerValid( From 3823ae9808972d2b391491c6d81c84396c4c0dc8 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 1 Sep 2026 23:16:57 -0700 Subject: [PATCH 2/6] Add tests for explicit routine target picking (CL-7355) Replaces the old chat-participant-inference tests with coverage that no target is ever inferred from a workbench's agents, that create sends the explicitly picked definitionAssetId, and that the picker's loading/empty/ error/stale states hold. --- apps/web/test/routine-panel.test.tsx | 279 ++++++++++++++++++--------- 1 file changed, 187 insertions(+), 92 deletions(-) diff --git a/apps/web/test/routine-panel.test.tsx b/apps/web/test/routine-panel.test.tsx index bc11a7731..1a9a1554e 100644 --- a/apps/web/test/routine-panel.test.tsx +++ b/apps/web/test/routine-panel.test.tsx @@ -1,13 +1,15 @@ // The routine panel (CL-6125, reworked CL-6139, trimmed to editor-only by -// CL-6362): create/edit one routine, inline in the canvas column — the -// back chevron closes the canvas, never a route hop. Browsing/running -// existing routines lives on the global `/routines` page now. Every write -// autosaves and is serialized through one queue (`saveState` shows -// "Saving…"/"Saved"/an honest error). A routine created from the panel -// always targets the conversation it was opened beside — that workbench's -// own host agent and its own id as the delivery destination — or, with no -// workbench in scope, this workbench's existing Myra workbench; never a -// newly minted one. +// CL-6362; target inference replaced by an explicit picker in CL-7355): +// create/edit one routine, inline in the canvas column — the back chevron +// closes the canvas, never a route hop. Browsing/running existing routines +// lives on the global `/routines` page now. Every write autosaves and is +// serialized through one queue (`saveState` shows "Saving…"/"Saved"/an +// honest error). A routine's delivery destination is the conversation the +// panel was opened beside — its own id, or, with no workbench in scope, +// this workbench's existing Myra workbench; never a newly minted one. What +// the routine *runs* is a separate, explicit pick from +// `GET /api/tenants/:tenantId/workflows/targets` — never inferred from the +// conversation's own agent. import { afterEach, beforeEach, describe, expect, test } from "bun:test"; @@ -44,6 +46,16 @@ function jsonResponse(body: unknown): Response { }); } +type TargetFixture = { + definitionAssetId: string; + definitionId: string; + assetName: string; + name: string; + description: string | null; + kind: "agent" | "workflow"; + wireHash: string; +}; + let routines: Record[] = []; let createdRoutine: Record | null = null; let updatedPatches: Record[] = []; @@ -54,24 +66,27 @@ let slackConfigured = false; let granolaConnected = false; let capabilitiesProbeFails = false; let networkDelayMs = 0; -let workbenchAgentsByWorkbench: Record< - string, +let targets: TargetFixture[] = [ { - address: string; - handle: string; - definitionId: string; - definitionAssetId: string; - }[] -> = { - ch_1: [ - { - address: "myra_1@wf_1.tnt_1", - handle: "myra", - definitionId: "wfd_1", - definitionAssetId: "wfd_1", - }, - ], -}; + definitionAssetId: "asset_myra", + definitionId: "wfd_1", + assetName: "myra", + name: "Myra", + description: "This workbench's own assistant.", + kind: "agent", + wireHash: "hash_1", + }, + { + definitionAssetId: "asset_digest", + definitionId: "wfd_2", + assetName: "digest-workflow", + name: "Morning digest workflow", + description: "Summarizes overnight activity.", + kind: "workflow", + wireHash: "hash_2", + }, +]; +let targetsRequestFails = false; let chatWorkbenches: Record[] = []; let runsByRoutineId: Record[]> = {}; let topLevelRuns: Record[] = []; @@ -83,7 +98,7 @@ function routineRecord( return { id: "rtn_1", name: "Morning digest", - definitionAssetId: "wfd_1", + definitionAssetId: "asset_myra", definitionId: "wfd_1", trigger: null, scope: "personal", @@ -138,17 +153,14 @@ async function routeFetch( if (url.includes("/credentials/resolve/")) { return new Response(null, { status: 404 }); } - if (url.includes("/workflows/definitions")) { - return jsonResponse({ - data: [{ id: "wfd_myra", name: "assistant", status: "deployed" }], - nextCursor: null, - }); - } - const agentsMatch = url.match(/\/chat\/workbenches\/([^/]+)\/agents$/); - if (agentsMatch) { - return jsonResponse({ - items: workbenchAgentsByWorkbench[agentsMatch[1] as string] ?? [], - }); + if (url.includes("/workflows/targets")) { + if (targetsRequestFails) { + return new Response( + JSON.stringify({ error: { userMessage: "Not authorized." } }), + { status: 403, headers: { "content-type": "application/json" } }, + ); + } + return jsonResponse({ items: targets, nextCursor: null }); } if ( url.includes("/chat/workbenches") && @@ -177,17 +189,6 @@ async function routeFetch( updatedAt: "2026-01-01T00:00:00.000Z", }; chatWorkbenches = [...chatWorkbenches, workbench]; - workbenchAgentsByWorkbench = { - ...workbenchAgentsByWorkbench, - ch_myra_new: [ - { - address: "myra_2@wf_2.tnt_1", - handle: "myra", - definitionId: "wfd_myra", - definitionAssetId: "wfd_myra", - }, - ], - }; return jsonResponse(workbench); } if (url.includes("/webhook-triggers") && method === "POST") { @@ -258,7 +259,6 @@ async function routeFetch( id: `rtn_${createRoutineCalls.length}`, name: body["name"], definitionAssetId: body["definitionAssetId"], - definitionId: body["definitionAssetId"], deliveryWorkbenchId: body["deliveryWorkbenchId"] ?? null, trigger: body["trigger"] ?? null, input: body["input"] ?? {}, @@ -294,20 +294,31 @@ describe("RoutinePanel", () => { granolaConnected = false; capabilitiesProbeFails = false; networkDelayMs = 0; + targetsRequestFails = false; + targets = [ + { + definitionAssetId: "asset_myra", + definitionId: "wfd_1", + assetName: "myra", + name: "Myra", + description: "This workbench's own assistant.", + kind: "agent", + wireHash: "hash_1", + }, + { + definitionAssetId: "asset_digest", + definitionId: "wfd_2", + assetName: "digest-workflow", + name: "Morning digest workflow", + description: "Summarizes overnight activity.", + kind: "workflow", + wireHash: "hash_2", + }, + ]; chatWorkbenches = []; runsByRoutineId = {}; topLevelRuns = []; runTraces = {}; - workbenchAgentsByWorkbench = { - ch_1: [ - { - address: "myra_1@wf_1.tnt_1", - handle: "myra", - definitionId: "wfd_1", - definitionAssetId: "wfd_1", - }, - ], - }; toastMock.mockClear(); }); @@ -393,6 +404,22 @@ describe("RoutinePanel", () => { }); } + function selectTarget(definitionAssetId: string) { + const select = container.querySelector( + "#routine-panel-target", + ) as HTMLSelectElement; + const setter = Object.getOwnPropertyDescriptor( + window.HTMLSelectElement.prototype, + "value", + )?.set; + if (setter === undefined) + throw new Error("native value setter unavailable"); + act(() => { + setter.call(select, definitionAssetId); + select.dispatchEvent(new Event("change", { bubbles: true })); + }); + } + describe("shared canvas-pane chrome (CL-6200)", () => { test("the editor view renders through the shared CanvasPaneHeader, not a hand-rolled one", async () => { await renderPanel({ routineId: null, workbenchId: "ch_1" }); @@ -419,20 +446,85 @@ describe("RoutinePanel", () => { expect(closed).toBe(true); }); - test("creating a routine targets the panel's own workbench: its host agent, and delivers back into it", async () => { + // CL-7355: no target is ever inferred from the conversation's own + // agent — typing a name and blurring with no target picked must not + // create anything. + test("no target is inferred from chat participants: naming a routine with nothing picked never creates", async () => { + await renderPanel({ routineId: null, workbenchId: "ch_1" }); + + const name = fieldByLabel("Name this routine") as HTMLInputElement; + fillAndBlur(name, "Morning digest"); + await settle(); + + expect(createRoutineCalls).toHaveLength(0); + }); + + test("picking a target, then naming the routine, creates with the picked definitionAssetId", async () => { await renderPanel({ routineId: null, workbenchId: "ch_1" }); + await settle(); + + selectTarget("asset_digest"); + await settle(); const name = fieldByLabel("Name this routine") as HTMLInputElement; fillAndBlur(name, "Morning digest"); await settle(); expect(createRoutineCalls).toHaveLength(1); - expect(createRoutineCalls[0]?.["definitionAssetId"]).toBe("wfd_1"); + expect(createRoutineCalls[0]?.["definitionAssetId"]).toBe( + "asset_digest", + ); expect(createRoutineCalls[0]?.["deliveryWorkbenchId"]).toBe("ch_1"); expect(toastMock).toHaveBeenCalled(); }); - test("no workbench in scope: falls back to the workbench's existing Myra workbench, never minting a new one", async () => { + test("groups targets by kind (Agents / Workflows) and lists both", async () => { + await renderPanel({ routineId: null, workbenchId: "ch_1" }); + await settle(); + + const select = container.querySelector( + "#routine-panel-target", + ) as HTMLSelectElement; + const groupLabels = [...select.querySelectorAll("optgroup")].map( + (group) => group.getAttribute("label"), + ); + expect(groupLabels).toEqual(["Agents", "Workflows"]); + const optionLabels = [...select.querySelectorAll("option")].map( + (option) => option.textContent, + ); + expect(optionLabels).toContain("Myra"); + expect(optionLabels).toContain("Morning digest workflow"); + }); + + test("no target is preselected — the picker opens with nothing chosen", async () => { + await renderPanel({ routineId: null, workbenchId: "ch_1" }); + await settle(); + const select = container.querySelector( + "#routine-panel-target", + ) as HTMLSelectElement; + expect(select.value).toBe(""); + }); + + test("empty target list shows the empty state with a link to Agents settings, not a picker", async () => { + targets = []; + await renderPanel({ routineId: null, workbenchId: "ch_1" }); + await settle(); + expect(container.querySelector("#routine-panel-target")).toBeNull(); + expect(container.textContent).toContain( + "No deployable workflows yet — author or install one", + ); + expect(buttonWithText("Go to Agents")).toBeDefined(); + }); + + test("a failed targets fetch shows an honest inline error, not a silent empty picker", async () => { + targetsRequestFails = true; + await renderPanel({ routineId: null, workbenchId: "ch_1" }); + await settle(); + expect(container.querySelector("#routine-panel-target")).toBeNull(); + expect(container.querySelector('[role="alert"]')).not.toBeNull(); + }); + + test("no workbench in scope: falls back to the workbench's existing Myra workbench for delivery, never minting a new one", async () => { chatWorkbenches = [ { id: "ch_myra", @@ -444,18 +536,11 @@ describe("RoutinePanel", () => { updatedAt: "2026-01-01T00:00:00.000Z", }, ]; - workbenchAgentsByWorkbench = { - ...workbenchAgentsByWorkbench, - ch_myra: [ - { - address: "myra_9@wf_9.tnt_1", - handle: "myra", - definitionId: "wfd_myra", - definitionAssetId: "wfd_myra", - }, - ], - }; await renderPanel({ routineId: null }); + await settle(); + + selectTarget("asset_myra"); + await settle(); const name = fieldByLabel("Name this routine") as HTMLInputElement; fillAndBlur(name, "Nightly summary"); @@ -463,12 +548,15 @@ describe("RoutinePanel", () => { expect(createRoutineCalls).toHaveLength(1); expect(createRoutineCalls[0]?.["deliveryWorkbenchId"]).toBe("ch_myra"); - expect(createRoutineCalls[0]?.["definitionAssetId"]).toBe("wfd_myra"); + expect(createRoutineCalls[0]?.["definitionAssetId"]).toBe("asset_myra"); expect(createWorkbenchCalls).toHaveLength(0); }); test("rapid Name and Instruction blur in the same tick serialize into one create, then one update — never two creates", async () => { await renderPanel({ routineId: null, workbenchId: "ch_1" }); + await settle(); + selectTarget("asset_myra"); + await settle(); const name = fieldByLabel("Name this routine") as HTMLInputElement; const instruction = fieldByLabel( @@ -504,8 +592,11 @@ describe("RoutinePanel", () => { }); test("shows Saving… while a write is in flight, then Saved", async () => { - networkDelayMs = 30; await renderPanel({ routineId: null, workbenchId: "ch_1" }); + await settle(); + selectTarget("asset_myra"); + await settle(); + networkDelayMs = 30; const name = fieldByLabel("Name this routine") as HTMLInputElement; const setter = Object.getOwnPropertyDescriptor( window.HTMLInputElement.prototype, @@ -525,21 +616,6 @@ describe("RoutinePanel", () => { expect(container.textContent).toContain("Saved"); }); - test("an honest inline error when the write fails", async () => { - await renderPanel({ routineId: null }); - // No workbenchId and no Myra workbench exists, and no assistant - // definition is deployed for this fixture tenant either — the - // fallback fails honestly rather than silently minting anything. - const originalDefs = workbenchAgentsByWorkbench; - workbenchAgentsByWorkbench = { ...originalDefs, ch_myra_new: [] }; - - const name = fieldByLabel("Name this routine") as HTMLInputElement; - fillAndBlur(name, "Morning digest"); - await settle(); - - expect(container.querySelector('[role="alert"]')).not.toBeNull(); - }); - test("Active toggle is optimistic — flips immediately, before the PATCH resolves", async () => { createdRoutine = routineRecord({ enabled: false }); routines = [createdRoutine]; @@ -555,6 +631,16 @@ describe("RoutinePanel", () => { expect(updatedPatches).toContainEqual({ enabled: true }); }); + test("existing-routine mode shows the current target's name, read-only", async () => { + createdRoutine = routineRecord({ definitionAssetId: "asset_digest" }); + routines = [createdRoutine]; + await renderPanel({ routineId: "rtn_1" }); + await settle(); + + expect(container.querySelector("#routine-panel-target")).toBeNull(); + expect(container.textContent).toContain("Morning digest workflow"); + }); + test("Test run is disabled until the routine is saved, then fires the run-once call", async () => { await renderPanel({ routineId: null, workbenchId: "ch_1" }); expect(buttonWithText("Run now")?.hasAttribute("disabled")).toBe(true); @@ -645,6 +731,9 @@ describe("RoutinePanel", () => { test("picking a schedule preset commits the trigger in one click — no sub-menu chain", async () => { await renderPanel({ routineId: null, workbenchId: "ch_1" }); + await settle(); + selectTarget("asset_myra"); + await settle(); act(() => openMenu(buttonWithText("+ Add trigger"))); await settle(); const onSchedule = [ @@ -678,6 +767,9 @@ describe("RoutinePanel", () => { // schedule picked while instruction was mid-edit). test("committing a schedule does not wipe an in-progress instruction that was never blurred", async () => { await renderPanel({ routineId: null, workbenchId: "ch_1" }); + await settle(); + selectTarget("asset_myra"); + await settle(); const name = fieldByLabel("Name this routine") as HTMLInputElement; fillAndBlur(name, "Morning digest"); @@ -714,8 +806,11 @@ describe("RoutinePanel", () => { }); test("typing instruction while name create is in flight survives the create ack", async () => { - networkDelayMs = 40; await renderPanel({ routineId: null, workbenchId: "ch_1" }); + await settle(); + selectTarget("asset_myra"); + await settle(); + networkDelayMs = 40; const name = fieldByLabel("Name this routine") as HTMLInputElement; const instruction = fieldByLabel( From 23ce35179852413cc9b00a0e8d00357b494f994d Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 1 Sep 2026 23:17:03 -0700 Subject: [PATCH 3/6] Replace routine-panel target inference with an explicit definition picker (CL-7355) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The routine panel used to derive a create target from the conversation's own agent (listWorkbenchAgents(...)[0]) — silently wrong the moment a workbench hosted more than one agent, or none, and it could never target a workflow. DefinitionTargetPicker instead lists every deployed, frozen target from GET /api/tenants/:tenantId/workflows/targets, grouped by kind (Agents / Workflows), and never auto-selects one. Create now requires a picked target; delivery (which workbench the routine posts back into) stays independent, unchanged from before. Existing-routine mode shows the current target's name read-only — editing it is CL-7358. --- .../src/shell/definition-target-picker.tsx | 170 ++++++++++++++++++ apps/web/src/shell/routine-panel.tsx | 161 +++++++++++------ 2 files changed, 274 insertions(+), 57 deletions(-) create mode 100644 apps/web/src/shell/definition-target-picker.tsx diff --git a/apps/web/src/shell/definition-target-picker.tsx b/apps/web/src/shell/definition-target-picker.tsx new file mode 100644 index 000000000..f0f1d89f7 --- /dev/null +++ b/apps/web/src/shell/definition-target-picker.tsx @@ -0,0 +1,170 @@ +// An explicit picker over `GET /api/tenants/:tenantId/workflows/targets` +// (CL-7355): the routine panel used to infer a create target from the +// conversation's own agent (`listWorkbenchAgents(...)[0]`) — silently wrong +// the moment a workbench hosted more than one agent, or none. This picker +// instead lists every deployed, frozen definition the signed-in principal +// may target, grouped presentationally by `kind` (Agents / Workflows), and +// leaves the choice to the person — it never auto-selects the first item. +import { useEffect, useState } from "react"; +import { Button, EmptyState, Select, Skeleton } from "@corbits/react-ui"; +import { Robot } from "@corbits/icons"; + +import { useNavigate } from "../navigation"; +import { listAllRoutineTargets } from "../routines-api"; +import type { RoutineTarget } from "../routines-api"; + +type LoadState = + | { readonly kind: "loading" } + | { readonly kind: "error"; readonly message: string } + | { readonly kind: "loaded"; readonly targets: readonly RoutineTarget[] }; + +/** `value`/`onChange` carry a `definitionAssetId` — the stable identity a + * routine stores. `preselectedAssetId` lets a caller that already knows + * which target it wants seed the initial selection explicitly; the picker + * itself never guesses one on the person's behalf. */ +export function DefinitionTargetPicker({ + tenantId, + value, + onChange, + preselectedAssetId, +}: { + readonly tenantId: string | null; + readonly value: string | null; + readonly onChange: (definitionAssetId: string) => void; + readonly preselectedAssetId?: string; +}) { + const navigate = useNavigate(); + const [state, setState] = useState({ kind: "loading" }); + + useEffect(() => { + if (tenantId === null) return; + let cancelled = false; + setState({ kind: "loading" }); + void listAllRoutineTargets(tenantId).then( + (targets) => { + if (cancelled) return; + setState({ kind: "loaded", targets }); + if ( + preselectedAssetId !== undefined && + value === null && + targets.some((t) => t.definitionAssetId === preselectedAssetId) + ) { + onChange(preselectedAssetId); + } + }, + (cause: unknown) => { + if (!cancelled) { + setState({ + kind: "error", + message: cause instanceof Error ? cause.message : String(cause), + }); + } + }, + ); + return () => { + cancelled = true; + }; + }, [tenantId]); + + if (state.kind === "loading") { + return ( +
+ What should this routine run? + +
+ ); + } + + if (state.kind === "error") { + return ( +
+ What should this routine run? +

+ {state.message} +

+
+ ); + } + + const { targets } = state; + + if (targets.length === 0) { + return ( + } + title="No deployable workflows yet — author or install one" + action={ + + } + /> + ); + } + + const nameCounts = new Map(); + for (const target of targets) { + nameCounts.set(target.name, (nameCounts.get(target.name) ?? 0) + 1); + } + const labelFor = (target: RoutineTarget): string => + (nameCounts.get(target.name) ?? 0) > 1 + ? `${target.name} (${target.assetName})` + : target.name; + + const agents = targets.filter((t) => t.kind === "agent"); + const workflows = targets.filter((t) => t.kind === "workflow"); + const selected = targets.find((t) => t.definitionAssetId === value) ?? null; + const stale = value !== null && selected === null; + + return ( +
+ + + {stale ? ( +

+ This routine's target is no longer available — pick a new one. +

+ ) : selected?.description !== null && selected?.description !== undefined ? ( +

{selected.description}

+ ) : null} +
+ ); +} diff --git a/apps/web/src/shell/routine-panel.tsx b/apps/web/src/shell/routine-panel.tsx index 20a3dc100..3535374df 100644 --- a/apps/web/src/shell/routine-panel.tsx +++ b/apps/web/src/shell/routine-panel.tsx @@ -20,16 +20,15 @@ // picking a schedule while another field's save is in flight keeps the // in-progress values. // -// A routine created from this panel always targets the conversation it -// was opened beside: every workbench's host participant is Myra, so "this -// workbench's own agent" resolves to that workbench's host agent -// (`listWorkbenchAgents`), and the routine delivers back into that same -// workbench — never a new one. A panel opened with no workbench in scope (a -// deliberate `/routines` visit) falls back to the workbench's own default -// Myra workbench (`ensureMyraWorkbench`, the one deliberate find-or-create -// path in the product) rather than the old tenant-wide "assistant" -// definition lookup, which had no workbench to deliver into at all and -// silently minted a fresh one server-side whenever delivery was required. +// The routine's delivery destination is the conversation this panel was +// opened beside — its own id is where the routine delivers back into. A +// panel opened with no workbench in scope (a deliberate `/routines` visit) +// falls back to the workbench's own default Myra workbench +// (`ensureMyraWorkbench`, the one deliberate find-or-create path in the +// product). What the routine *runs* is a separate, explicit choice (CL-7355): +// the panel no longer infers it from the conversation's own agent — a +// person picks a target from `DefinitionTargetPicker`, backed by +// `GET /api/tenants/:tenantId/workflows/targets`. import { useEffect, useRef, useState } from "react"; import type { ChangeEvent } from "react"; import { useQueryClient } from "@tanstack/react-query"; @@ -47,7 +46,6 @@ import { Textarea, toast, } from "@corbits/react-ui"; -import { listWorkbenchAgents } from "@corbits/chat-ui"; import { Clock, X } from "@corbits/icons"; import { useBench } from "../bench-context"; @@ -55,17 +53,24 @@ import { useNavigate } from "../navigation"; import { ensureMyraWorkbench } from "../myra-workbench"; import { routineScheduleSentence } from "@corbits/routines/client"; import { ScheduleEditor } from "../routine-schedule"; +import { DefinitionTargetPicker } from "./definition-target-picker"; import { createRoutine, deleteRoutine, getRoutine, + listAllRoutineTargets, listRoutineRuns, routineCreatedToast, routineRunStartedToast, runRoutineNow, updateRoutine, } from "../routines-api"; -import type { Routine, RoutineRun, RoutineTrigger } from "../routines-api"; +import type { + Routine, + RoutineRun, + RoutineTarget, + RoutineTrigger, +} from "../routines-api"; import { RunsTable } from "../pages/routines-page"; import { createWebhookTrigger, @@ -177,11 +182,6 @@ export function RoutinePanel() { type SaveState = "idle" | "saving" | "saved" | "error"; -type CreateTarget = { - readonly definitionAssetId: string; - readonly deliveryWorkbenchId: string; -}; - /** The editor view: create/edit one routine. Self-fetching — handed only * the subject, loads the rest itself, exactly like `ProfileCanvasPane`. */ function RoutineEditorPanel({ @@ -222,6 +222,10 @@ function RoutineEditorPanel({ const [saveState, setSaveState] = useState("idle"); const [error, setError] = useState(null); const [runs, setRuns] = useState([]); + const [targetAssetId, setTargetAssetId] = useState(null); + const [existingTarget, setExistingTarget] = useState( + null, + ); // Every write (create or update) this panel session makes runs through // this one chain — never two in flight at once. `routineIdRef` is the @@ -229,6 +233,8 @@ function RoutineEditorPanel({ // resolves, so a second commit queued before the first finished sees the // *post*-first-write id, not a stale snapshot from before either ran. const routineIdRef = useRef(routineId); + const targetAssetIdRef = useRef(targetAssetId); + targetAssetIdRef.current = targetAssetId; const writeChainRef = useRef>(Promise.resolve()); const draftRef = useRef({ name: "", @@ -270,6 +276,8 @@ function RoutineEditorPanel({ setSaveState("idle"); setError(null); setRuns([]); + setTargetAssetId(null); + setExistingTarget(null); }, [subject]); const loadRuns = (id: string) => { @@ -292,6 +300,7 @@ function RoutineEditorPanel({ setTrigger(routine.trigger); setSavedTrigger(routine.trigger); setEnabled(routine.enabled); + setTargetAssetId(routine.definitionAssetId); draftRef.current = { name: routine.name, savedName: routine.name, @@ -359,37 +368,43 @@ function RoutineEditorPanel({ }; }, [tenantId, subject.routineId]); - /** This routine's own agent + delivery workbench: the conversation the - * panel was opened beside (its host participant — every workbench's - * host is Myra), or, with no conversation in scope, this workbench's - * own default Myra workbench. Never mints a new workbench — `ensureMyraWorkbench` - * finds-or-creates the one singleton Myra conversation this tenant - * already has. */ - const resolveCreateTarget = async (): Promise => { + // Existing-routine mode shows the current target's display name, + // read-only for this issue — editing it is CL-7358. A stale target (no + // longer in the tenant's deployable list) is shown honestly as its raw + // id rather than hidden. + useEffect(() => { + if (tenantId === null || subject.routineId == null) return; + let cancelled = false; + void listAllRoutineTargets(tenantId).then( + (targets) => { + if (cancelled) return; + setExistingTarget( + targets.find((t) => t.definitionAssetId === targetAssetId) ?? null, + ); + }, + () => { + if (!cancelled) setExistingTarget(null); + }, + ); + return () => { + cancelled = true; + }; + }, [tenantId, subject.routineId, targetAssetId]); + + /** This routine's delivery workbench: the conversation the panel was + * opened beside (its own id), or, with no conversation in scope, this + * workbench's own default Myra workbench. Never mints a new workbench — + * `ensureMyraWorkbench` finds-or-creates the one singleton Myra + * conversation this tenant already has. Delivery is independent of what + * the routine runs (CL-7355) — that's `targetAssetId`, picked explicitly. */ + const resolveDeliveryWorkbenchId = async (): Promise => { if (tenantId === null) { throw new Error("No workbench to create this in yet"); } - if (subject.workbenchId !== undefined) { - const workbenchId = subject.workbenchId; - const agents = await listWorkbenchAgents(tenantId, workbenchId); - const definitionAssetId = agents[0]?.definitionAssetId; - if (definitionAssetId === undefined) { - throw new Error( - "This conversation has no agent to run this routine yet.", - ); - } - return { definitionAssetId, deliveryWorkbenchId: workbenchId }; - } + if (subject.workbenchId !== undefined) return subject.workbenchId; const result = await ensureMyraWorkbench(tenantId); if (result.kind === "error") throw new Error(result.message); - const agents = await listWorkbenchAgents(tenantId, result.workbenchId); - const definitionAssetId = agents[0]?.definitionAssetId; - if (definitionAssetId === undefined) { - throw new Error( - "This workbench has no assistant to run this routine yet.", - ); - } - return { definitionAssetId, deliveryWorkbenchId: result.workbenchId }; + return result.workbenchId; }; /** Every create/update this panel makes funnels through this one chain — @@ -421,11 +436,15 @@ function RoutineEditorPanel({ readonly instruction: string; readonly trigger: RoutineTrigger; }): Promise => { - const target = await resolveCreateTarget(); + const definitionAssetId = targetAssetIdRef.current; + if (definitionAssetId === null) { + throw new Error("Pick what this routine runs before saving."); + } + const deliveryWorkbenchId = await resolveDeliveryWorkbenchId(); const routine = await createRoutine(tenantId as string, { name: fields.name, - definitionAssetId: target.definitionAssetId, - deliveryWorkbenchId: target.deliveryWorkbenchId, + definitionAssetId, + deliveryWorkbenchId, scope: "personal", trigger: fields.trigger, runOnceNow: false, @@ -447,10 +466,14 @@ function RoutineEditorPanel({ // write's result, so it correctly updates instead. The bail-outs below // (blank name, unchanged value) are safe to read eagerly — being a tick // stale there costs at most one redundant PATCH, never a second POST. + // A brand-new routine requires a picked target before its first write can + // fire at all (CL-7355) — a blur with no target selected yet is not a + // failed save, it's simply not ready to submit. const commitName = () => { const trimmed = name.trim(); if (trimmed === "") return; if (routineIdRef.current !== null && trimmed === savedName) return; + if (routineIdRef.current === null && targetAssetIdRef.current === null) return; void runWrite((id) => { if (id === null) return doCreate({ name: trimmed, instruction, trigger }); return updateRoutine(tenantId as string, id, { name: trimmed }); @@ -461,6 +484,7 @@ function RoutineEditorPanel({ const trimmed = instruction.trim(); if (routineIdRef.current === null && name.trim() === "") return; if (routineIdRef.current !== null && trimmed === savedInstruction) return; + if (routineIdRef.current === null && targetAssetIdRef.current === null) return; void runWrite((id) => { if (id === null) { return doCreate({ name: name.trim(), instruction, trigger }); @@ -476,6 +500,7 @@ function RoutineEditorPanel({ draftRef.current.trigger = next; setTriggerSourceLabel(null); setAddingSchedule(false); + if (routineIdRef.current === null && targetAssetIdRef.current === null) return; void runWrite((id) => { if (id === null) { return doCreate({ @@ -488,6 +513,15 @@ function RoutineEditorPanel({ }); }; + // Picking a target never itself submits — it only clears the "no target + // yet" bail-out so the next field blur (Name/Instruction) can create. + // `targetAssetIdRef` is set synchronously here so a blur landing in the + // very same tick already sees the pick, not a stale pre-render snapshot. + const pickTarget = (definitionAssetId: string) => { + targetAssetIdRef.current = definitionAssetId; + setTargetAssetId(definitionAssetId); + }; + const removeTrigger = () => { setTrigger(null); draftRef.current.trigger = null; @@ -508,23 +542,19 @@ function RoutineEditorPanel({ let targetRoutineId = id; let definitionAssetId: string; if (targetRoutineId === null) { - const target = await resolveCreateTarget(); - const created = await createRoutine(tenantId, { + const created = await doCreate({ name: name.trim() || "Untitled routine", - definitionAssetId: target.definitionAssetId, - deliveryWorkbenchId: target.deliveryWorkbenchId, - scope: "personal", + instruction, trigger: null, - runOnceNow: false, - ...(instruction.trim() !== "" - ? { input: { instruction: instruction.trim() } } - : {}), }); targetRoutineId = created.id; definitionAssetId = created.definitionAssetId; - toast(routineCreatedToast(created.name)); } else { - definitionAssetId = (await resolveCreateTarget()).definitionAssetId; + const existing = targetAssetIdRef.current; + if (existing === null) { + throw new Error("Pick what this routine runs before saving."); + } + definitionAssetId = existing; } const binding = await createWebhookTrigger(tenantId, { name: `${name.trim() || "Untitled routine"} — ${sourceLabel}`, @@ -628,6 +658,23 @@ function RoutineEditorPanel({ /> + {routineId === null ? ( + + ) : ( +
+ + What this routine runs + +
+ {existingTarget?.name ?? targetAssetId ?? "—"} +
+
+ )} +