v2: surface UI — events, workflow triggers, team ownership, shared parts - #216
v2: surface UI — events, workflow triggers, team ownership, shared parts#216xBalbinus wants to merge 15 commits into
Conversation
# Conflicts: # packages/web/src/api/client.ts
There was a problem hiding this comment.
The PR is in good shape overall — the new API routes, wire types, query hooks, and UI components are internally consistent and well-tested. Two correctness issues are worth fixing before merge.
-
packages/api/src/routes/workflows.ts:~316 —deleteWorkflowSchedulereturns 200 whether the schedule existed or not, but the existence check above already verified it; the concern is the opposite direction: ifdeleteWorkflowScheduleinternally no-ops (e.g. concurrent delete), the response still saysdeleted: true. More critically: after the belongs-to-workflow list check passes,deleteWorkflowScheduleis called with onlyowner.orgIdandscheduleId— it does not re-verify the schedule belongs to this workflow at the service level. The guard is a list-then-delete with no lock: a TOCTOU window exists where a concurrent request could reassign or re-use thescheduleIdbetween thelistWorkflowSchedulesread and thedeleteWorkflowSchedulewrite. This is only exploitable if the service allows schedule IDs to be reused or transferred, which may be unlikely in practice, but the route should passworkflowIdinto the delete call (or the service should accept it) so theWHEREclause isscheduleId = ? AND workflowId = ?instead of justscheduleId = ?.- Consider adding
workflowIdtodeleteWorkflowSchedule's signature/query to make the constraint atomic.
- Consider adding
-
packages/web/src/components/workflows/webhook-section.tsx:~34 —window.location.originis used to construct the webhook URL displayed to users. In environments where the web app is proxied or served on a different origin than the API (common in dev and some production setups), this will silently produce a URL that doesn't work. The API host should come from the same configuration source asapi/client.ts'sAPIbase URL, not fromwindow.location.origin. -
packages/web/src/components/settings/teams-panel.test.tsx:~56-58 — the test suite re-renders into the same container acrossitblocks without abeforeEach(() => { document.body.innerHTML = ''; })orcleanup(). ThecallerRole/orgRolemutation-between-tests pattern combined with no DOM cleanup means earlier renders can bleed into later assertions (screen.queryByRolesearches the entire document). This may not manifest today because the mocked data drivescallerRolebeforerender, but it's fragile — a failed assertion can leave stale DOM. Use@testing-library/react'scleanup(automatic with vitest-jsdom when configured, but worth verifying it's wired).
Created on behalf of Xiangan He xiangan@turnkey.io
- Validate schedule name (trim, reject blank) and input (must be an object) instead of persisting junk into every scheduled run's trigger payload. - Delete a workflow's schedules when the workflow itself is deleted, matching the existing webhook cleanup. - Scope the schedule DELETE route's ownership check to a WHERE predicate instead of an org-wide scan-and-.some(). - Dedupe the webhook URL builder (webhookUrl in actions.ts duplicated workflowWebhookUrl in webhook-service.ts); the HTTP routes now return the same absolute url the agent-facing tool already computed, with the request's own origin as a fallback when no public origin is configured.
PATCH/DELETE on a personal (user-owned) subscription now 404 for any caller but its creator, matching the cross-owner "not found" convention used elsewhere. Previously any org member could enable/disable or delete a colleague's personal subscription through the org-scoped routes. An org-owned subscription stays mutable by any org member.
dev-seed-linear.ts passed PGlite straight to PgCredentialStore, whose PgQueryable contract requires query() to return rowCount — PGlite's result type doesn't have one, so this broke the repo's `tsc --build`. Adapts through a thin query wrapper instead. dev-events-smoke.ts probed three guessed shapes for the webhook mint response (hookId/secret/parsed-from-url); imports the real WorkflowWebhookResponse type instead.
New reusable pieces, extracted after the events+triggers+teams panels independently reinvented the same patterns: - OwnerPicker: the caller-or-team select used by the workflow and skill editors. Filters to teams the caller can see AND is a member of (callerRole !== null) — the prior two copies each offered every team an org admin's useTeams() returns, including ones they aren't a member of, which the create routes then 404 on. - SelectMenu: the trigger-button-plus-option-list pattern behind the event feed's service/key filters and the subscription dialog's workflow picker (three near-identical DropdownMenu blocks before this). - useCopyToClipboard: extracted from tool-shell's CopyButton (which now uses it too) so the workflow webhook section doesn't hand-roll its own copy-state timer, and so both handle a denied/unavailable clipboard. - TabBar: adds the ARIA tabs keyboard contract (arrow keys move focus and selection with roving tabindex, Home/End jump to the ends) and an aria-controls/tabPanelId pairing for consumers to wire a real tabpanel.
- new-workflow-dialog and skill-editor now share OwnerPicker instead of each keeping its own copy-pasted <select>, and route their create errors through errorText instead of showing raw error.message. - Webhook rotate/delete: the ConfirmDialogs now receive the mutation error (previously invisible, rendered below the modal overlay) and the initial "Create webhook URL" failure has its own error line. useMintWorkflowWebhook seeds the query cache from its response so a rotate doesn't keep showing the just-revoked URL until the refetch lands; useDeleteWorkflowWebhook invalidates onSettled so a failed delete still reconciles. - Schedule delete now confirms (it was one click, unlike every other destructive control added this pass) and surfaces its error; the empty-state message no longer renders alongside the error message on a failed fetch; useDeleteWorkflowSchedule invalidates onSettled.
mutations by owner, close remaining error/reset gaps - Feed's two filter dropdowns and the create dialog's workflow picker now use SelectMenu instead of three copies of the same DropdownMenu block. Added a truncation notice when the feed is at the server's 50-event page size (no pagination yet) instead of silently looking complete. The load-failure message now names the Refresh button as the corrective action. - listEvents built its query string with a URLSearchParams.size check — unsupported by some engines, which would silently drop the service/key filters; switched to .toString(). - The subscriptions list disables the enable/disable switch and hides the actions menu for a subscription the caller isn't allowed to mutate (mirrors the new server-side gate), and the switch surfaces a failed PATCH instead of silently snapping back. - The create dialog now resets its form and error state on any close path (Cancel, overlay, Escape), not just a successful submit — it stays mounted between opens. - teams-panel: dropped a dead Spinner import and routed its two raw error.message displays through errorText. Also adds the ARIA tabpanel half of the /events tab strip (role, aria-labelledby) to match TabBar's new aria-controls wiring.
|
1. Schedule delete scoping — partly acted on, in The cited concern is already closed at HEAD: There was a real residue one line below the cited spot, though: the check was scoped but the The route guard is owner/team scoped, which is strictly stronger than the org scope described. 2. Webhook URL from 3. DOM cleanup in Full api suite green (1979 passed). |
…ent-native run status Splits the 522-line node inspector into one form file per node type (mirrors the tool-renderer registry pattern). Consolidates four duplicate date formatters onto lib/format-when.ts and three hand-rolled clipboard handlers onto lib/use-copy.ts (upgraded with the execCommand fallback one of them had, so nothing regresses). Upgrades the run-detail checkpoint list from a plain-text status list to color/glyph-coded rows sharing the canvas's own NodeRunStatus vocabulary, with failed nodes auto-expanding their result and everything else collapsed by default. Gives the pending- approval card real visual prominence instead of a plain bordered box. Demotes the editor toolbar's least-used action (version history) into an overflow menu, cutting the primary row from 4 buttons to 3.
Summary
One PR for the V2 surface gaps — concepts that had full APIs and no UI — consolidating #214 and #215, plus a componentization pass so the new panels share one set of parts instead of each hand-rolling their own.
Events (
/events, new top-level page):Workflow triggers (editor → Triggers drawer):
GET/POST /api/workflows/:id/schedulesandDELETE /api/workflows/:id/schedules/:scheduleIdover the existing schedule service. Deleting the workflow deletes its schedules too. Delete confirms, like every other destructive control here.Teams:
TeamSummary.callerRole; the teams settings panel hides mutation controls the API'scanMutateTeamgate would 404 anyway. No enforcement semantics changed.Shared components (new, used across all of the above):
ConfirmDialog,LoadingRow/ErrorRow/EmptyRow,TabBar(full ARIA tabs keyboard contract — arrow keys, Home/End, roving tabindex),SelectMenu(the trigger-button-plus-option-list pattern three different filters/pickers were each reinventing),OwnerPicker(caller-or-team select, filtered to teams the caller can actually create against),useCopyToClipboard,~/lib/format-when,~/lib/error-text. Panels are split into focused files rather than one growing god-component: event row/detail, subscription create dialog, webhook and schedule sections.Dev tooling:
packages/api/scripts/dev-seed-linear.ts+dev-events-smoke.tsdrive the full webhook → event → subscription → run pipeline against a local stack.Fixed after the first pass
An internal review pass caught 15 real issues in the initial diff — API validation gaps (a schedule accepted a whitespace-only name and a non-object
input), an authorization gap (any org member could mutate a colleague's personal event subscription through the org-scoped routes), missing error surfaces on four different destructive/mutating controls, an orphaned-schedule bug on workflow delete, a duplicated webhook-URL builder, and the reuse gaps the componentization above closes. All fixed in this branch; see individual commits.Verification
API (1979) and web (612) suites pass; full workspace typecheck clean. Exercised live end to end on the dev stack: created a team-owned workflow through the fixed OwnerPicker, opened its Triggers drawer, minted a webhook (confirmed the
urlround-trips), rotated it (confirmed the cache updates immediately, not after a refetch), created and deleted a schedule through its confirm dialog, and confirmed the DELETE requests land.Brand refresh (2026-08-11)
Retargeted the design-token layer onto a real brand identity — decision record:
docs/specs/2026-08-11-brand-refresh-design.md. One neutral system (cool OKLCH gray, replacing a separate warm palette), one accent (blue, sampled from the mascot illustration's uniform — extended to a full 50–900 scale), the unused serif heading face dropped for the sans stack it was already silently falling back to, wash colors rebuilt in nativeoklch(... / alpha)so they can't drift from their base color, plus a real favicon and theme-color meta tags. Two files changed the whole app's palette; verified live in light and dark across Events, the workflow editor, and Teams settings.Codebase cleanup + agent-native run status (2026-08-11)
Full audit pass: zero orphan component files found, but four duplicate date-formatting functions and three hand-rolled clipboard-copy handlers had crept back in around this PR's own extractions — consolidated onto
lib/format-when.ts/lib/use-copy.ts(the clipboard hook picked up anexecCommandfallback one of the three copies had and the others didn't, so nothing lost capability).Split the 522-line node inspector (
editor/inspector.tsx) intoeditor/node-forms/— one file per node type plus a registry dispatcher, same pattern the session tool-renderers already use. No behavior change; same 13 tests pass.Two components pulled from the referenced "AI-native interfaces" gallery, scoped to what Valet's workflow surface actually needed: the run-detail checkpoint list now reads as color/glyph-coded status rows (same vocabulary the canvas already uses) instead of plain text, with a failed node's result auto-expanded and everything else collapsed; the pending-approval card gets real visual weight (accent-tinted, "Waiting on you" framing) instead of a plain bordered box.
Editor toolbar: demoted "History" (version restore — the one setup-once, rarely-needed action of the four) into an overflow menu, cutting the primary row to Runs / Triggers / Run.
Component patterns + team-oriented surfaces (2026-08-12)
Two follow-ups against the reference component gallery and the team-ownership work above.
Component patterns:
Thinking—engineToWirePartspreviously dropped the model's thinking parts; now forwarded through the wire and rendered as a collapsed-by-default disclosure in the transcript.CodeBlock— one shared syntax-highlighted renderer for markdown-fenced code, replacing the ad-hoc<pre>treatmentmarkdown.tsxhad; selectively registers only the languages this product renders (shell, TS/JS/TSX/JSX, JSON, diff, YAML, SQL, Markdown, Python) instead of Prism's full ~290-language bundle, and colors tokens from the app's own light/dark CSS variables rather than a fixed prebuilt theme. Elapsed-time counter on the agent status badge, sourced from the server-stamped turn-start event rather than the client clock.Two other reference patterns were scoped out for lack of backend support: Streaming Text's inline sources/follow-ups, and the Prompt Bar's @-mentions/slash-commands/dictation. Both need server-side data this API doesn't produce yet.
Team-oriented resources: skill sources can now be imported as team-owned (same
OwnerPickerthe workflow/skill editors already use), with a badge on the row showing which team owns it.Team orchestrators were structurally real already — workflow dispatch could wake one — but nothing let a member view it: nothing created its app-row on first access, and the session read routes only checked direct ownership. Adds
POST /api/teams/:id/orchestrator(get-or-create, reusing the existingensureOrchestratorSessionhelper) and widensGET /api/sessions/:id, the messages/threads/decisions routes, and the WS handshake to also allow a caller who can view the owning team. Read-only and team-scoped — no change to mutating session routes, no org-level access.TeamsPanelgets an "Assistant" button, visible to any member, that opens the team's orchestrator session.Also fixed:
createWorkflowSchedulealways wroteownerType: "user"regardless of who actually owned the workflow being scheduled — now derives it from the workflow's real owner (team ownership maps to org; the schedule-owner enum has no team value).Verification (2026-08-12)
API (1993 passing, 30 pre-existing skips) and web (644) suites pass; full workspace typecheck clean. Live-verified on the dev stack in both light and dark:
CodeBlock's syntax highlighting end to end through a real chat turn, and the team-orchestrator flow — clicking Assistant creates the session and navigates to it, the session renders through the widened read routes, and a second click returns the same session (get-or-create is idempotent).Team surfaces moved out of Settings (2026-08-12)
A team's assistant was reachable only at Settings › Organization › Teams, on a small ghost button, behind an org-admin gate most members fail. Settings is where you configure things; that is the wrong place to keep a conversation.
Each team you belong to now has a permanent row in the
/chatsidebar, beside your own assistant, addressed by/chat?team=<id>. Finding your team's assistant is the same act as finding your own — two clicks for any member. This follows Notion's teamspace pattern rather than a switcher or a separate destination: membership is navigation, not a mode, so no view re-scopes and every surface stays as cross-team as it was.The session id is derived client-side from the engine's well-known format, so browsing the rail creates nothing —
/chatensures the session only when someone opens the conversation. A strip above the composer names how many people can read what you are about to type.Supporting changes:
OwnerBadge, linking into that team's assistant, so flat lists become a second way in./chatrather than a competing door. The org-admin gate is unchanged.Nothing renders for a caller with no teams: the rail block, the dashboard card, and the badges all require the organizations feature on and at least one real membership. A solo user's app is byte-identical to before.
Two bugs this surfaced
The header lied. It derived "is an orchestrator" from a bare
id.startsWith("orchestrator:")and then titled the session from the viewer's own assistant, so every member saw their personal assistant's name on a shared team conversation. It now resolves the team and marks the session as shared.Lifecycle authorization ignored the team.
PATCH /api/sessions/:id,POST /:id/pauseandDELETE /:idall filtered onagent_sessions.userId, which on a team-owned row records whoever opened the assistant first. That member could pause or delete an agent the whole team shares; everyone else got a 404. AddscanAdministerSession(team-owned → team admin or org admin) as the deliberate mirror ofcanViewSession, and extractscanAdministerTeamso the team mutation routes and session administration share one definition.POST /:id/auto-titlehad the same filter, which left a member's threads permanently untitled — titling is part of prompting, so it moves tocanViewSession.Known gaps
A team assistant has no chosen name, personality, presence dot, or nested child sessions, because
GET /api/orchestrator/infoand/childrenresolve the caller's own principal. It carries the team's name fromGET /api/teamsand its threads work fully. Nested children are deliberately omitted in team scope rather than shown wrong.Verification
API (2014 passing, 30 pre-existing skips) and web (674) suites pass; workspace typecheck clean. Exercised live on the dev stack in light and dark: opened a team assistant from the rail, the dashboard card, and a workflow's owner badge; confirmed the shared-with strip, the corrected title, the thread list switching scope, and that an unknown
?team=falls back to the personal assistant with a notice.