v2: triggers UI, team-owned workflow creation, role-aware teams panel - #215
v2: triggers UI, team-owned workflow creation, role-aware teams panel#215xBalbinus wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
The PR is well-structured and the implementation matches the stated intent. One correctness issue in the DELETE schedule route and one potential UX race condition are worth addressing before merge.
-
packages/api/src/routes/workflows.ts:325— schedule ownership check is read-then-delete, not atomic, and leaks the schedule's existence across orgsThe DELETE handler calls
listWorkflowSchedules(deps.db, owner.orgId, id)to verify the schedule belongs to the workflow, then callsdeleteWorkflowSchedule(deps.db, owner.orgId, scheduleId)separately.deleteWorkflowScheduleis org-scoped but not workflow-scoped — if the schedule was reassigned between the list and delete (unlikely but possible under concurrent requests), the invariant breaks. More importantly, the list filters byworkflowId, so a schedule belonging to a different workflow in the same org would correctly 404 here — butdeleteWorkflowSchedulewould still delete it if it somehow received a stalescheduleId. The cleaner fix is fordeleteWorkflowScheduleto accept an optionalworkflowIdconstraint and apply it in the SQLWHERE, making the check atomic. As written the window is small but the belt-and-suspenders comment in the PR description implies this was meant to be air-tight. -
packages/web/src/components/workflows/triggers-drawer.tsx:66—window.location.originin a SSR-friendly codebase- The webhook URL is constructed as
`${window.location.origin}/api/hooks/…`. If this component is ever rendered server-side (or in tests withoutjsdom), this will throwReferenceError: window is not defined. The test file uses// @vitest-environment jsdomso tests pass, but if the route is SSR-rendered before hydration, it will blow up. A safe guard would betypeof window !== "undefined" ? window.location.origin : ""or pulling the origin from a config/env variable consistent with how the API base URL is resolved elsewhere inclient.ts.
- The webhook URL is constructed as
-
packages/web/src/components/workflows/new-workflow-dialog.tsx:97— the owner picker shows all org teams, not just teams the caller belongs toThe
GET /api/teamsendpoint for a non-admin returns only the caller's own teams (listTeamsForUser), but for an org admin it returns every team in the org (line 155 ofteams.ts). So an org admin who is not a member of a team will see it in the picker and can attempt to create a workflow owned by that team — the API's membership check will then reject it with a 404/403. This is a usability gap: the picker should filter toteams.data?.teams.filter(t => t.callerRole !== null)sincecallerRole === nullmeans "you're an org admin viewing a team you're not on." ThecallerRolefield was added specifically to enable this kind of gate.
Created on behalf of Xiangan He xiangan@turnkey.io
Summary
Three V2 surface gaps closed in one pass — thin UI, plus two thin routes, over enforcement the API already has:
schedule-service.ts, swept by the scheduler) but no HTTP surface — this addsGET/POST /api/workflows/:id/schedulesandDELETE /api/workflows/:id/schedules/:scheduleIdover it. The routes resolve the workflow throughgetWorkflowDefinitionfirst (own-rows 404 convention), and a delete requires the schedule to belong to that workflow, so one workflow's surface cannot remove another's rows.CreateWorkflowRequest.teamIdexisted on the wire with membership checks in the API, but no UI offered it. The New-workflow dialog gains an owner picker (you, or a team you belong to), and the workflows list badges team-owned rows with the team name.TeamSummarygainscallerRole("admin" | "member" | null — null is an org admin viewing a team they are not on). The teams settings panel now hides mutation controls (delete team, add/remove member, role change) from callers the API'scanMutateTeamgate would 404 anyway. No enforcement changed; the API checks stay authoritative.The spec addendum is in
docs/specs/2026-07-16-workflows-overhaul-design.md.Verification
API and web suites pass (1966 api / 593 web). Exercised live on the dev stack: created a team, created a team-owned workflow through the dialog (
ownerType: teamconfirmed server-side), minted the webhook URL from the drawer and started a run by POSTing to that exact URL (run settled), and created a schedule (correct next-fire in America/New_York) that the drawer lists.