From e7c6f85553bd8efb0f7af0f488f615ac93c496ae Mon Sep 17 00:00:00 2001 From: Guilhem Date: Fri, 11 Sep 2026 15:47:15 +0200 Subject: [PATCH 01/12] feat: give the chat the full MCP tool schema, and mark calls with the provider icon (#11086) * feat: mark MCP server lists and chat tool calls with the provider icon Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JgzuxyafKNF2uaEeL35XQw * feat: return the full MCP tool schema from search_mcp_tools Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JgzuxyafKNF2uaEeL35XQw * fix: bound an empty MCP search result and keep the server mark decorative Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JgzuxyafKNF2uaEeL35XQw * fix: keep the more-matches hint and scope a marked row to its own workspace Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JgzuxyafKNF2uaEeL35XQw --------- Co-authored-by: Claude Opus 5 (1M context) --- .../copilot/chat/AssistantMcpSection.svelte | 13 ++-- .../copilot/chat/ToolExecutionDisplay.svelte | 19 +++++ .../copilot/chat/global/mcpTools.test.ts | 62 ++++++++++++++- .../copilot/chat/global/mcpTools.ts | 76 +++++++++++-------- .../src/lib/components/copilot/chat/shared.ts | 5 ++ .../lib/components/mcp/McpServerIcon.svelte | 29 +++++++ frontend/src/lib/components/mcp/iconCache.ts | 12 +++ .../src/lib/components/mcp/mcpMenu.svelte.ts | 13 +--- .../src/lib/components/mcp/serverMark.test.ts | 45 +++++++++++ frontend/src/lib/components/mcp/serverMark.ts | 47 ++++++++++++ 10 files changed, 273 insertions(+), 48 deletions(-) create mode 100644 frontend/src/lib/components/mcp/McpServerIcon.svelte create mode 100644 frontend/src/lib/components/mcp/serverMark.test.ts create mode 100644 frontend/src/lib/components/mcp/serverMark.ts diff --git a/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte b/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte index 1637165f87..0ab2812b2e 100644 --- a/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte +++ b/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte @@ -20,6 +20,7 @@ switch that decides whether this chat carries its tools. forgetProviderKey, rememberProviderKey } from '$lib/components/mcp/iconCache' + import McpServerIcon from '$lib/components/mcp/McpServerIcon.svelte' import ConfirmationModal from '$lib/components/common/confirmationModal/ConfirmationModal.svelte' import type { Component } from 'svelte' import { ResourceService } from '$lib/gen' @@ -29,6 +30,7 @@ switch that decides whether this chat carries its tools. import { untrack } from 'svelte' import { getAiChatManager } from './aiChatManagerContext' import { clearMcpToolsCache } from './global/mcpTools' + import { forgetMcpServerMarks } from '$lib/components/mcp/serverMark' let { ws, @@ -389,8 +391,10 @@ switch that decides whether this chat carries its tools. async function refresh(target = ws) { // A path can be reconnected to a different server, so the cached tool list - // (and the readOnlyHint the confirmation gate reads) must not survive. + // (and the readOnlyHint the confirmation gate reads) must not survive — nor the + // provider mark the transcript's call rows show. clearMcpToolsCache() + forgetMcpServerMarks() await loadServers(target) // `refreshMcpServers` blanks the list when the workspace it is handed is not // the one the chat is on, so a refresh landing after a switch would take B's @@ -469,12 +473,7 @@ switch that decides whether this chat carries its tools.
{#each servers as server (server.path)} {#snippet icon()} - {#if server.icon} - {@const Icon = server.icon} - - {:else} - - {/if} + {/snippet} {#snippet title()} {server.path} diff --git a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte index f5ae22a332..620357ff87 100644 --- a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte @@ -40,6 +40,8 @@ import RunScriptCard from './RunScriptCard.svelte' import WebSearchSourcesDisplay from './WebSearchSourcesDisplay.svelte' import ExpandableImage from '$lib/components/common/image/ExpandableImage.svelte' + import McpServerIcon from '$lib/components/mcp/McpServerIcon.svelte' + import { resolveMcpServerMark } from '$lib/components/mcp/serverMark' interface Props { message: ToolDisplayMessage @@ -47,6 +49,11 @@ let { message }: Props = $props() + // Recorded by the call itself, from the connected-server list rather than from the + // model's arguments — which is what lets a reloaded transcript still resolve it, and + // what keeps a path the model made up from being read as a workspace resource. + const mcpServer = $derived(message.mcpServer) + const isPlanReview = $derived(message.toolName === EXIT_PLAN_MODE_TOOL) const isPlanCard = $derived(isPlanCardTool(message.toolName)) const planCopy = $derived( @@ -242,6 +249,17 @@ {/if} {/snippet} + + {#snippet serverMark()} + {#if mcpServer?.workspace && mcpServer.path} + {#await resolveMcpServerMark(mcpServer.workspace, mcpServer.path) then mark} + + {/await} + {/if} + {/snippet} + @@ -258,6 +276,7 @@ labelClass={showPreviewChip ? 'truncate' : ''} contentClass="space-y-3" headerRight={showPreviewChip ? previewChip : undefined} + headerLeft={mcpServer?.workspace ? serverMark : undefined} > {#snippet belowHeader()} diff --git a/frontend/src/lib/components/copilot/chat/global/mcpTools.test.ts b/frontend/src/lib/components/copilot/chat/global/mcpTools.test.ts index a155115d28..e5d3af8b36 100644 --- a/frontend/src/lib/components/copilot/chat/global/mcpTools.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/mcpTools.test.ts @@ -228,7 +228,7 @@ describe('call results', () => { }) describe('search_mcp_tools', () => { - it('returns compact summaries without the full input schemas', async () => { + it('returns each match with its full input schema', async () => { const result = await run('search_mcp_tools', { query: 'issue' }) expect(result.matches).toEqual([ { @@ -236,7 +236,7 @@ describe('search_mcp_tools', () => { tool: 'get_issue', description: 'Get details of a GitHub issue', mode: 'read', - params: ['owner', 'repo'] + inputSchema: TOOLS[0].inputSchema } ]) }) @@ -330,6 +330,64 @@ describe('result size cap', () => { expect(callMcpToolMock).not.toHaveBeenCalled() }) + it('drops the lowest-ranked schemas before dropping the matches themselves', async () => { + getMcpToolsMock.mockResolvedValue( + ['issue_a', 'issue_b', 'issue_c'].map((name) => ({ + name, + description: 'An issue tool', + inputSchema: { + type: 'object', + properties: { body: { type: 'string', description: 'x'.repeat(9_000) } } + }, + annotations: { readOnlyHint: true } + })) + ) + const result = await run('search_mcp_tools', { query: 'issue' }) + + expect(result.matches.map((m: any) => m.tool)).toEqual(['issue_a', 'issue_b', 'issue_c']) + expect(result.matches[0].inputSchema).toBeDefined() + expect(result.matches[2].inputSchema).toBeUndefined() + expect(result.truncated).toBe(true) + }) + + // Truncating must not swallow the count of matches the score cut off: those + // tools are not in the result at all, so the model has to know to ask again. + it('keeps the more-matches hint when truncation strips schemas', async () => { + getMcpToolsMock.mockResolvedValue( + Array.from({ length: 12 }, (_, i) => ({ + name: `issue_${i}`, + description: 'An issue tool', + inputSchema: { + type: 'object', + properties: { body: { type: 'string', description: 'x'.repeat(3_000) } } + }, + annotations: { readOnlyHint: true } + })) + ) + const result = await run('search_mcp_tools', { query: 'issue' }) + + expect(result.matches).toHaveLength(10) + expect(result.note).toContain('2 more match(es)') + expect(result.note).toContain('inputSchema') + }) + + it('holds the cap when every server failed and nothing matched', async () => { + getMcpToolsMock.mockRejectedValue(new Error('x'.repeat(500))) + const servers = Array.from({ length: 50 }, (_, i) => ({ path: `u/hugo/mcp_${i}` })) + const raw = await createMcpTools(servers) + .find((t) => t.def.function.name === 'search_mcp_tools')! + .fn({ + args: { query: 'issue' }, + workspace: 'test-ws', + helpers: {}, + toolCallbacks: createToolCallbacks(), + toolId: 'tool-1' + }) + + expect(raw.length).toBeLessThanOrEqual(20_000) + expect(JSON.parse(raw).unavailableCount).toBe(50) + }) + it('truncates an oversized tools/list failure in search', async () => { getMcpToolsMock.mockRejectedValue(new Error('x'.repeat(80_000))) const result = await run('search_mcp_tools', { query: 'issue' }) diff --git a/frontend/src/lib/components/copilot/chat/global/mcpTools.ts b/frontend/src/lib/components/copilot/chat/global/mcpTools.ts index d5f16d5c54..6fd59e4a07 100644 --- a/frontend/src/lib/components/copilot/chat/global/mcpTools.ts +++ b/frontend/src/lib/components/copilot/chat/global/mcpTools.ts @@ -8,8 +8,7 @@ import { enabledMcpPaths } from '$lib/components/mcp/enabledServers' * as three static tools — search, read call, write call — instead of one * registered tool per remote tool. A server like GitHub's exposes ~90 tools, * whose schemas would otherwise be re-sent on every chat iteration; here only - * matched summaries enter the model's context, and a full input schema only - * after a call fails. + * the tools a search matched enter the model's context, with their schemas. */ type McpToolDef = GetMcpToolsResponse[number] @@ -17,7 +16,7 @@ type McpToolDef = GetMcpToolsResponse[number] export type McpServer = { path: string; editedAt?: string } const MAX_SEARCH_RESULTS = 10 -const MAX_DESCRIPTION_CHARS = 200 +const MAX_DESCRIPTION_CHARS = 1_000 const MAX_RESULT_CHARS = 20_000 // A server writes its own error text, and every enabled server can contribute // one, so search results are capped the same way call results are. @@ -139,24 +138,17 @@ function isReadOnly(tool: McpToolDef): boolean { return tool.annotations?.readOnlyHint === true } -function schemaPropertyNames(schema: unknown): string[] { - const properties = (schema as { properties?: Record } | null | undefined) - ?.properties - return properties ? Object.keys(properties) : [] -} - function truncate(text: string, max: number): string { return text.length > max ? text.slice(0, max) + '…' : text } function summarizeTool(server: McpServer, tool: McpToolDef) { - const params = schemaPropertyNames(tool.inputSchema) return { server: server.path, tool: tool.name, description: truncate(tool.description ?? '', MAX_DESCRIPTION_CHARS), mode: isReadOnly(tool) ? 'read' : 'write', - ...(params.length > 0 ? { params } : {}) + inputSchema: tool.inputSchema } } @@ -285,28 +277,44 @@ function bounded(payload: { } /** - * The search payload under the same ceiling as a call result. Server error text - * goes first: the matches are what the model asked for. + * The search payload under the same ceiling as a call result. Schemas go before + * matches: a tool listed without its schema is still callable, since a rejected + * call returns the schema, while a tool dropped from the list is not. */ function boundedSearch(payload: { matches: unknown[]; [k: string]: unknown }): string { - const { unavailable, ...rest } = payload - const dropped = Array.isArray(unavailable) ? { unavailableCount: unavailable.length } : {} - let out = JSON.stringify(payload, null, 2) + let out = JSON.stringify(payload) if (out.length <= MAX_RESULT_CHARS) return out - const matches = [...payload.matches] - const build = () => - JSON.stringify( - { - ...rest, - ...dropped, - matches, - truncated: true, - note: `Truncated to ${MAX_RESULT_CHARS} characters. Refine the query.` - }, - null, - 2 - ) + + // The caller's note says how many matches the score cut off, which truncating + // only adds to. + const { unavailable, note: scoreNote, ...rest } = payload + const dropped = Array.isArray(unavailable) ? { unavailableCount: unavailable.length } : {} + const found = payload.matches.length + const matches = payload.matches.map((m) => ({ ...(m as object) })) as Record[] + const build = () => { + const schemaless = matches.filter((m) => !m.inputSchema).length + const note = typeof scoreNote === 'string' ? [scoreNote] : [] + note.push(`Truncated to ${MAX_RESULT_CHARS} characters.`) + if (schemaless > 0) { + note.push( + `${schemaless} lower-ranked match(es) are listed without their inputSchema; calling one returns the schema if the arguments are wrong.` + ) + } + if (matches.length < found) { + note.push(`${found - matches.length} match(es) dropped — refine the query to see them.`) + } + return JSON.stringify({ ...rest, ...dropped, matches, truncated: true, note: note.join(' ') }) + } + // Dropping the server error text is the first reduction, and the only one left + // when nothing matched. out = build() + if (out.length <= MAX_RESULT_CHARS) return out + // `matches` is ordered by score, so the tail is what the query matched least. + for (let i = matches.length - 1; i >= 0; i--) { + delete matches[i].inputSchema + out = build() + if (out.length <= MAX_RESULT_CHARS) return out + } while (out.length > MAX_RESULT_CHARS && matches.length > 0) { matches.pop() out = build() @@ -353,6 +361,14 @@ function createCallTool(servers: McpServer[], mode: 'read' | 'write'): Tool<{}> }), fn: async ({ args, workspace, toolId, toolCallbacks }) => { const parsed = callMcpToolSchema.parse(args) + // Marks the row with the provider it reaches. Recorded before the listing is + // awaited, so an unreachable server still marks its own failure row, and taken + // from the connected list rather than from `parsed.server`, which is a path the + // model could point anywhere. + const named = servers.find((s) => s.path === parsed.server) + if (named) { + toolCallbacks.setToolStatus(toolId, { mcpServer: { workspace, path: named.path } }) + } // Listing is a live call to a third party: a server that has gone away // must fail this tool, not the chat loop around it. let resolved: Awaited> @@ -405,7 +421,7 @@ export function createMcpTools(servers: McpServer[]): Tool<{}>[] { def: createToolDef( searchMcpToolsSchema, 'search_mcp_tools', - 'Search the tools exposed by the MCP servers connected to this workspace (listed in the system prompt). Returns server + tool names to pass to call_mcp_read_tool or call_mcp_write_tool.' + 'Search the tools exposed by the MCP servers connected to this workspace (listed in the system prompt). Returns server + tool names to pass to call_mcp_read_tool or call_mcp_write_tool, each with the input schema its arguments must follow.' ), fn: async ({ args, workspace, toolId, toolCallbacks }) => { const parsed = searchMcpToolsSchema.parse(args) diff --git a/frontend/src/lib/components/copilot/chat/shared.ts b/frontend/src/lib/components/copilot/chat/shared.ts index adb0f63be4..bc00e415f6 100644 --- a/frontend/src/lib/components/copilot/chat/shared.ts +++ b/frontend/src/lib/components/copilot/chat/shared.ts @@ -629,6 +629,11 @@ export type ToolDisplayMessage = { autoCollapseDetails?: boolean isStreamingArguments?: boolean toolName?: string + /** What marks this row with its provider. Recorded rather than looked up, because the + * server listing lives only in memory and a reloaded transcript does not have it. The + * workspace rides along: a chat is readable from any workspace, and the same path names + * a different server in each. */ + mcpServer?: { workspace: string; path: string } showFade?: boolean actions?: ToolDisplayAction[] userQuestion?: UserQuestionDisplay diff --git a/frontend/src/lib/components/mcp/McpServerIcon.svelte b/frontend/src/lib/components/mcp/McpServerIcon.svelte new file mode 100644 index 0000000000..b3d005ad7f --- /dev/null +++ b/frontend/src/lib/components/mcp/McpServerIcon.svelte @@ -0,0 +1,29 @@ + + + + diff --git a/frontend/src/lib/components/mcp/iconCache.ts b/frontend/src/lib/components/mcp/iconCache.ts index e3c0f592e2..59ba63adfb 100644 --- a/frontend/src/lib/components/mcp/iconCache.ts +++ b/frontend/src/lib/components/mcp/iconCache.ts @@ -33,6 +33,18 @@ export function cachedProviderKey( return entry.editedAt === editedAt ? entry.key : undefined } +/** + * The stored mark for a path, ignoring `editedAt`: a transcript row has none to + * match against, and nothing acts on a mark. + */ +export function cachedProviderMark( + workspace: string, + path: string +): { key: string | null } | undefined { + const entry = read()[workspace]?.[path] + return entry ? { key: entry.key } : undefined +} + export function rememberProviderKey( workspace: string, path: string, diff --git a/frontend/src/lib/components/mcp/mcpMenu.svelte.ts b/frontend/src/lib/components/mcp/mcpMenu.svelte.ts index 847c03e61a..aea4a7b114 100644 --- a/frontend/src/lib/components/mcp/mcpMenu.svelte.ts +++ b/frontend/src/lib/components/mcp/mcpMenu.svelte.ts @@ -1,4 +1,4 @@ -import { List, Plug, Plus } from 'lucide-svelte' +import { List, Plus } from 'lucide-svelte' import type { Component } from 'svelte' import { get } from 'svelte/store' import { ResourceService } from '$lib/gen' @@ -9,6 +9,7 @@ import type { AIChatManager } from '../copilot/chat/AIChatManager.svelte' import { isMcpEnabled, setMcpEnabled } from './enabledServers' import { cachedProviderKey, rememberProviderKey } from './iconCache' import { loadProviderIcon } from './providerIcon' +import McpServerIcon from './McpServerIcon.svelte' type Row = { path: string @@ -164,19 +165,13 @@ export class McpMenu { return [ ...shown.map(({ path }) => ({ displayName: path, + icon: McpServerIcon, // Getters, not snapshots: the menu stays open across a click, and it has // to read through the live list rather than the row captured here, since // a reload replaces every row object and a getter bound to the old one // would go on reporting the state it was built with. - get icon() { - // Plug where the provider is unknown, so one nameless server does not - // pull its label out of line with the rest. - return row(path)?.icon ?? Plug - }, - // Provider icons take css lengths and ignore lucide's `size`, so without - // this one of them renders at its 24px default among 14px menu icons. get iconProps() { - return row(path)?.icon ? { width: '14px', height: '14px' } : undefined + return { icon: row(path)?.icon, size: 14 } }, get toggle() { return row(path)?.enabled ?? false diff --git a/frontend/src/lib/components/mcp/serverMark.test.ts b/frontend/src/lib/components/mcp/serverMark.test.ts new file mode 100644 index 0000000000..7e34d27284 --- /dev/null +++ b/frontend/src/lib/components/mcp/serverMark.test.ts @@ -0,0 +1,45 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { getResourceMock } = vi.hoisted(() => ({ getResourceMock: vi.fn() })) + +vi.mock('$lib/gen', () => ({ ResourceService: { getResource: getResourceMock } })) +vi.mock('./iconCache', () => ({ cachedProviderMark: () => undefined })) +vi.mock('./providerIcon', () => ({ + loadProviderIcon: async (key: string | null) => (key ? `icon:${key}` : undefined), + providerKey: (url: unknown) => + typeof url === 'string' && url.includes('linear') ? 'linear' : null +})) + +import { forgetMcpServerMarks, resolveMcpServerMark } from './serverMark' + +describe('resolveMcpServerMark', () => { + beforeEach(() => { + forgetMcpServerMarks() + getResourceMock.mockReset() + }) + + it('reads a server once and shares the answer', async () => { + getResourceMock.mockResolvedValue({ value: { url: 'https://mcp.linear.app/mcp' } }) + + const marks = await Promise.all([ + resolveMcpServerMark('ws', 'u/admin/linear_mcp'), + resolveMcpServerMark('ws', 'u/admin/linear_mcp') + ]) + + expect(marks.map((m) => m.icon)).toEqual(['icon:linear', 'icon:linear']) + expect(getResourceMock).toHaveBeenCalledTimes(1) + }) + + // A failure must not be memoized: one offline blip would otherwise leave the + // server unmarked in every later row until the page reloads. + it('retries after a failed read', async () => { + getResourceMock.mockRejectedValueOnce(new Error('offline')) + expect(await resolveMcpServerMark('ws', 'u/admin/linear_mcp')).toEqual({}) + + getResourceMock.mockResolvedValue({ value: { url: 'https://mcp.linear.app/mcp' } }) + const mark = await resolveMcpServerMark('ws', 'u/admin/linear_mcp') + + expect(mark.icon).toBe('icon:linear') + expect(getResourceMock).toHaveBeenCalledTimes(2) + }) +}) diff --git a/frontend/src/lib/components/mcp/serverMark.ts b/frontend/src/lib/components/mcp/serverMark.ts new file mode 100644 index 0000000000..c728abdead --- /dev/null +++ b/frontend/src/lib/components/mcp/serverMark.ts @@ -0,0 +1,47 @@ +import type { Component } from 'svelte' +import { ResourceService } from '$lib/gen' +import { cachedProviderMark } from './iconCache' +import { loadProviderIcon, providerKey } from './providerIcon' + +/** Windmill's own icon for a connected server's integration, when it ships one. */ +export type McpServerMark = { icon?: Component } + +// One resolution per server per session, shared by every transcript row naming it — +// a chat can hold dozens of calls against the same server. +const marks = new Map>() + +export function resolveMcpServerMark(workspace: string, path: string): Promise { + const key = `${workspace}:${path}` + let pending = marks.get(key) + if (!pending) { + // A failed read is dropped rather than memoized: one offline blip or a 401 during + // a token refresh would otherwise leave the server unmarked in every later row + // until the page reloads. + pending = load(workspace, path).catch(() => { + marks.delete(key) + return {} + }) + marks.set(key, pending) + } + return pending +} + +/** + * Forget what was resolved, for the settings section to call when it reloads the + * connections: a path can be reconnected to a different provider, and a mark held for + * the life of the page would go on marking new call rows with the old provider's icon. + */ +export function forgetMcpServerMarks() { + marks.clear() +} + +async function load(workspace: string, path: string): Promise { + const cached = cachedProviderMark(workspace, path) + if (cached) return { icon: await loadProviderIcon(cached.key) } + // Deliberately not written back to the shared cache: that entry is keyed by + // `editedAt` for the server list's sake, and storing one from here — where the + // row is a past call and `editedAt` is unknown — would make every list re-read. + const resource = await ResourceService.getResource({ workspace, path }) + const url = (resource.value as { url?: unknown } | undefined)?.url + return { icon: await loadProviderIcon(providerKey(url)) } +} From b50de8947908f1a5a4e9472afe6c0ecd25892e93 Mon Sep 17 00:00:00 2001 From: AlexRV12 <71396855+AlexRV12@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:37:53 +0200 Subject: [PATCH 02/12] feat: run a deployed flow through the chat's argument form (#11085) * feat: run a deployed flow through the chat's argument form Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015vHs7Jr4UDSbUe2KGjugMw * refactor: drop the unread dynselect helper from the deployed flow run form Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015vHs7Jr4UDSbUe2KGjugMw * fix: skip the preprocessor when the chat runs a deployed flow Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015vHs7Jr4UDSbUe2KGjugMw * test: pin run_flow steering with ai_evals cases Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015vHs7Jr4UDSbUe2KGjugMw * refactor: inline the deployed flow schema and trim the eval draft check Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015vHs7Jr4UDSbUe2KGjugMw * docs: correct the stale draft-validation comment on the flow test-run eval Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015vHs7Jr4UDSbUe2KGjugMw --------- Co-authored-by: Claude Opus 5 (1M context) --- ai_evals/cases/global.yaml | 66 ++++++++++++++++++ .../global/initial/notify_customer_flow.json | 40 +++++++++++ .../chat/global/apiCatalogTools.test.ts | 46 +++++++++---- .../copilot/chat/global/apiCatalogTools.ts | 1 + .../copilot/chat/global/core.test.ts | 62 +++++++++++++++++ .../components/copilot/chat/global/core.ts | 67 ++++++++++++++++++- 6 files changed, 267 insertions(+), 15 deletions(-) create mode 100644 ai_evals/fixtures/frontend/global/initial/notify_customer_flow.json diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml index 16d96d10b9..71693156a9 100644 --- a/ai_evals/cases/global.yaml +++ b/ai_evals/cases/global.yaml @@ -2046,6 +2046,72 @@ - passes the account "acme" - does not invent or guess the token's value +- id: global-test35-run-deployed-flow-with-form + prompt: |- + Run the deployed flow `f/evals/global/notify_customer` for me — the customer is `acme`. + initial: ai_evals/fixtures/frontend/global/initial/notify_customer_flow.json + runtime: + maxTurns: 8 + # A session chat is where the run card has a preview pane beside it; run_flow + # itself is offered in every chat. + sessionChat: true + validate: + draftCountExactly: 0 + toolExpect: + requiredToolsUsed: + - run_flow + # A draft may declare different arguments than the deployed version being run, so + # the names to prefill have to come from the deployed schema. + - read_workspace_item + forbiddenToolsUsed: + - test_run_flow + - call_api_endpoint + - write_flow + - deploy_workspace_item + # An empty form pushes the work back onto the user, so the prefill is part of + # what the tool is for. + toolCallArgs: + - tool: run_flow + field: args.customer + stringIncludesAnyOf: + - acme + # Running produces no draft, and the judge cannot observe runs; validate via tool use. + skipJudge: true + judgeChecklist: + - runs the deployed flow through run_flow rather than a preview test run or a raw API endpoint + - passes the customer "acme" so the confirmation form comes up prefilled + +- id: global-test36-draft-flow-test-run-not-deployed + prompt: |- + Update the `calculate_total` step of `f/evals/global/process_invoice` so it applies 8% tax and + returns `subtotal`, `tax` and `total`, then run it to check it works. + Keep it as an AI draft only; do not deploy or save it. + initial: ai_evals/fixtures/frontend/global/initial/process_invoice_flow.json + runtime: + maxTurns: 10 + validate: + draftCountExactly: 1 + requiredDrafts: + - type: flow + path: f/evals/global/process_invoice + toolExpect: + # A one-step flow is as well checked by running the step as the whole flow, so both + # count: what matters is that the run is against the draft. + requiredToolsAnyOf: + - [test_run_flow, test_run_step] + # The draft is what the user asked to check, and run_flow would run the deployed + # version instead — the edit would not be in what ran. + forbiddenToolsUsed: + - run_flow + - call_api_endpoint + - deploy_workspace_item + # The judge cannot observe runs, and the edit's content is already pinned by + # global-test5 on this fixture; what this case guards is where the run went. + skipJudge: true + judgeChecklist: + - creates an AI draft of f/evals/global/process_invoice applying 8% tax + - does not deploy or save the draft + - id: global-undo-created-draft prompt: |- Create a draft Postgres resource at `u/admin/scratch_db` for host db.example.com port 5432, database `orders`, user `app`, and tell me what fields it ended up with. diff --git a/ai_evals/fixtures/frontend/global/initial/notify_customer_flow.json b/ai_evals/fixtures/frontend/global/initial/notify_customer_flow.json new file mode 100644 index 0000000000..d8cbf37ace --- /dev/null +++ b/ai_evals/fixtures/frontend/global/initial/notify_customer_flow.json @@ -0,0 +1,40 @@ +{ + "workspace": { + "flows": [ + { + "path": "f/evals/global/notify_customer", + "summary": "Notify a customer", + "description": "Sends a notification to the named customer.", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "customer": { + "type": "string" + } + }, + "required": ["customer"] + }, + "value": { + "modules": [ + { + "id": "notify", + "summary": "Send the notification", + "value": { + "type": "rawscript", + "language": "bun", + "content": "export async function main(customer: string) {\n return `Notified ${customer}`\n}\n", + "input_transforms": { + "customer": { + "type": "javascript", + "expr": "flow_input.customer" + } + } + } + } + ] + } + } + ] + } +} diff --git a/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.test.ts b/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.test.ts index deb3694e99..0b1ea0952e 100644 --- a/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.test.ts @@ -101,6 +101,19 @@ const CATALOG = [ }, body_schema: { type: 'object', additionalProperties: true } }, + { + name: 'cancelQueuedJob', + description: 'Cancel a queued job', + instructions: '', + path: '/w/{workspace}/jobs_u/queue/cancel/{id}', + method: 'POST', + path_params_schema: { + type: 'object', + properties: { workspace: { type: 'string' }, id: { type: 'string' } }, + required: ['workspace', 'id'] + }, + body_schema: { type: 'object', additionalProperties: true } + }, { name: 'runFlowByPath', description: 'Run flow by path', @@ -179,7 +192,7 @@ describe('call_api_get', () => { const unknown = await run('call_api_get', { name: 'nope' }) expect(unknown.error).toContain('search_api_endpoints') - const mutating = await run('call_api_get', { name: 'runFlowByPath' }) + const mutating = await run('call_api_get', { name: 'cancelQueuedJob' }) expect(mutating.error).toContain('call_api_endpoint') const deleting = await run('call_api_endpoint', { name: 'deleteSchedule' }) @@ -191,17 +204,22 @@ describe('call_api_get', () => { expect(search.matches.map((m: any) => m.name)).not.toContain('deleteScriptByHash') }) - // Left reachable, this endpoint is the way around the argument form: it runs the - // deployed script on the model's arguments, unstripped and unshown. - it('refuses a deployed script run, pointing at run_script', async () => { - const called = await run('call_api_endpoint', { name: 'runScriptByPath' }) - expect(called.error).toContain('run_script') - expect(called.success).toBe(false) + // Left reachable, these endpoints are the way around the argument form: they run the + // deployed runnable on the model's arguments, unstripped and unshown. + it('refuses a deployed run, pointing at run_script and run_flow', async () => { + for (const [name, tool, query] of [ + ['runScriptByPath', 'run_script', 'run deployed script'], + ['runFlowByPath', 'run_flow', 'run deployed flow'] + ]) { + const called = await run('call_api_endpoint', { name }) + expect(called.error).toContain(tool) + expect(called.success).toBe(false) - // And it is gone from search, so the model is redirected before it ever calls. - const search = await run('search_api_endpoints', { query: 'run deployed script' }) - expect(search.matches.map((m: any) => m.name)).not.toContain('runScriptByPath') - expect(search.covered_by_dedicated_tools?.join(' ')).toContain('run_script') + // And it is gone from search, so the model is redirected before it ever calls. + const search = await run('search_api_endpoints', { query }) + expect(search.matches.map((m: any) => m.name)).not.toContain(name) + expect(search.covered_by_dedicated_tools?.join(' ')).toContain(tool) + } }) it('refuses draft-blind item reads and lists, pointing at the draft-aware tools', async () => { @@ -259,11 +277,11 @@ describe('call_api_endpoint', () => { }) vi.stubGlobal('fetch', fetchMock) const result = await run('call_api_endpoint', { - name: 'runFlowByPath', - params: { path: 'u/me/myflow' }, + name: 'cancelQueuedJob', + params: { id: 'job/1' }, body: { args: { n: 1 } } }) - expect(fetchMock).toHaveBeenCalledWith('/api/w/test-ws/jobs/run/f/u%2Fme%2Fmyflow', { + expect(fetchMock).toHaveBeenCalledWith('/api/w/test-ws/jobs_u/queue/cancel/job%2F1', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ args: { n: 1 } }) diff --git a/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts b/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts index 2271ff91e0..42d45916eb 100644 --- a/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts +++ b/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts @@ -37,6 +37,7 @@ const COVERED_ENDPOINTS: Record = { listResource: 'list_workspace_items (it includes your drafts)', listSchedules: 'list_workspace_items (it includes your drafts)', runScriptByPath: 'run_script (it shows the user an argument form to confirm)', + runFlowByPath: 'run_flow (it shows the user an argument form to confirm)', deleteScriptByPath: 'delete_workspace_item', deleteScriptByHash: 'delete_workspace_item', deleteFlowByPath: 'delete_workspace_item', diff --git a/frontend/src/lib/components/copilot/chat/global/core.test.ts b/frontend/src/lib/components/copilot/chat/global/core.test.ts index d8abb38cdf..90ff8a8f77 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -5081,6 +5081,68 @@ describe('global AI tools', () => { }) }) + // What separates a deployed run from a test run of the same flow: the form is built from the + // deployed schema rather than the draft's, and the editor open on that path is left alone — + // it holds the draft, so a deployed run painted into its graph would show steps that are not + // the ones running. + it('run_flow forms on the deployed flow and leaves the live editor alone', async () => { + seedBackendDraft( + 'flow', + 'u/admin/deployed_and_drafted', + { + path: 'u/admin/deployed_and_drafted', + summary: 'Draft of the deployed flow', + value: { modules: [{ id: 'draft_step', value: { type: 'identity' } }] }, + schema: { type: 'object', properties: { draft_only: { type: 'string' } } }, + edited_by: '', + edited_at: '', + archived: false, + extra_perms: {} + }, + { workspace: WORKSPACE } + ) + UserDraft.setLiveEditorDraft({ + workspace: WORKSPACE, + itemKind: 'flow', + storagePath: 'u/admin/deployed_and_drafted', + effectivePath: 'u/admin/deployed_and_drafted' + }) + vi.mocked(FlowService.getFlowByPath).mockResolvedValueOnce({ + path: 'u/admin/deployed_and_drafted', + summary: 'Deployed flow', + value: { modules: [{ id: 'deployed_step', value: { type: 'identity' } }] }, + schema: FLOW_NAME_SCHEMA + } as any) + const testActiveFlow = vi.fn(async () => 'job-live-flow') + + let form: any + await withCompletedTestJob(() => + callGlobalTool( + 'run_flow', + { path: 'u/admin/deployed_and_drafted', args: { name: 'Ada' } }, + { + ...toolCallbacks, + requestRunArgs: async (_toolId, f) => { + form = f + return { name: 'Grace' } + } + }, + { testActiveFlow } + ) + ) + + expect(form.runnableKind).toBe('flow') + expect(form.schema?.properties).toEqual(FLOW_NAME_SCHEMA.properties) + expect(testActiveFlow).not.toHaveBeenCalled() + expect(JobService.runFlowPreview).not.toHaveBeenCalled() + expect(JobService.runFlowByPath).toHaveBeenCalledWith({ + workspace: WORKSPACE, + path: 'u/admin/deployed_and_drafted', + requestBody: { name: 'Grace' }, + skipPreprocessor: true + }) + }) + it('test_run_step previews rawscript steps from the draft flow', async () => { const content = 'export async function main(name: string) {\n\treturn name.toUpperCase()\n}' await callGlobalTool('write_flow', { diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index adb4f4e6e6..5883c5ae0d 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -935,6 +935,20 @@ const testRunFlowToolDef = createToolDef( { strict: false } ) +const runFlowSchema = z.object({ + path: z.string().describe('Workspace path of the deployed flow to run.'), + args: testRunArgsSchema, + background: backgroundArgSchema, + wait_seconds: waitSecondsArgSchema +}) + +const runFlowToolDef = createToolDef( + runFlowSchema, + 'run_flow', + 'Run a DEPLOYED flow for real, under the user\'s own permissions. Fill in every argument you can infer: the user gets an argument form prefilled with `args` and decides what runs. For a secret argument prefer `$var:` naming an existing workspace variable; a literal is minted into a short-lived secret before the run, but stays in this call. A required file is the user\'s to attach, so call this even when you cannot supply one rather than asking in chat. Use only when the user names the deployed version ("the deployed X", "in production", "for real"); otherwise use test_run_flow.', + { strict: false } +) + const testRunStepSchema = z.object({ path: z.string().describe('Workspace path of the flow containing the step to test.'), stepId: z.string().describe('The id of the step/module to test.'), @@ -1350,7 +1364,7 @@ ${pipelineBullet} : ' Pass items (":" entries naming the items you changed) so the review is scoped to them — omitting items preselects every pending change in the workspace' }, or mode ("draft" or "fork") to force which comparison is shown. Prefer offering this review page over calling deploy_workspace_item directly when several items changed. - For a Windmill operation no other tool covers (workers, queue state, a run's args, ...), use search_api_endpoints to find a REST endpoint, then call_api_get for reads or call_api_endpoint for mutations (the user is asked to confirm those). Always prefer a dedicated tool when one exists; endpoints for authoring or deleting scripts, flows, apps, schedules, resources, or variables are not available through the API catalog tools — use the draft tools and delete_workspace_item instead. -- Default to test_run_script, test_run_flow, or test_run_step for any run request, an existing script included; they prefer drafts and need no deployment. Use run_script only when the user names the deployed version ("the deployed X", "in production", "for real") — a bare "run X" is not that. For run_script, read the item with read_workspace_item version: "deployed" first so the arguments match the deployed schema, and fill in every one you can infer. test_run_script, run_script and test_run_flow all show the user an argument form prefilled with what you sent, so fill in every argument you can infer rather than asking for it in chat. runFlowByPath from the API catalog is the exception — it runs a deployed flow with no form at all: only for a flow the user asked to run deployed. +- Default to test_run_script, test_run_flow, or test_run_step for any run request, an existing script included; they prefer drafts and need no deployment. Use run_script or run_flow only when the user names the deployed version ("the deployed X", "in production", "for real") — a bare "run X" is not that. For those two, read the item with read_workspace_item version: "deployed" first so the arguments match the deployed schema. test_run_script, test_run_flow, run_script and run_flow all show the user an argument form prefilled with what you sent, so fill in every argument you can infer rather than asking for it in chat. - When a required decision is ambiguous, use askUserQuestion with two to ten clear proposed answer strings instead of guessing. The user can also type a custom answer when none of the proposed answers fit. Set multiSelect: true only when the answers can genuinely co-apply and the user may pick several (not mutually exclusive). - When the user asks you to remember a lasting preference, always/never do something, or change/stop a behavior going forward, call update_user_instructions to persist it. It edits only the USER INSTRUCTIONS block (not WORKSPACE INSTRUCTIONS). Keep each instruction concise; do not use it for one-off requests scoped to the current task. - Keep context targeted.${ @@ -3704,6 +3718,19 @@ export const globalTools: Tool<{}>[] = [ showDetails: true, autoCollapseDetails: false }, + { + def: runFlowToolDef, + fn: async (ctx) => { + const parsed = runFlowSchema.parse(ctx.args) + return runDeployedFlow(parsed, ctx) + }, + bypassedByAutoAccept: true, + streamingLabel: 'Preparing the run form...', + confirmationMessage: 'Run a deployed flow', + queuedLabel: (args) => `Run ${args?.path ?? 'a flow'}`, + showDetails: true, + autoCollapseDetails: false + }, { def: testRunStepToolDef, fn: async (ctx) => { @@ -5791,6 +5818,44 @@ async function runDeployedScript( ) } +async function runDeployedFlow( + args: z.infer, + ctx: WriteDraftCtx +): Promise { + const { workspace } = ctx + // No live editor is driven here as a test run drives one: that editor holds the draft, and a + // deployed run painted into its graph would show steps that are not the ones running. + const flow = await FlowService.getFlowByPath({ workspace, path: args.path }) + return runThroughForm( + { + path: args.path, + schema: (flow.schema as Record) ?? {}, + summary: flow.summary, + kind: 'run', + // No code/lang: the dynamic-option pickers come from the deployed flow, not an inline copy. + schemaNoun: 'deployed', + toolName: 'run_flow', + proposed: args.args, + startMessage: `Running "${args.path}"...`, + contextName: 'flow', + autoAcceptable: true, + background: args.background, + detachAfterMs: waitSecondsToDetachMs(args.wait_seconds), + startJob: (submitted) => + JobService.runFlowByPath({ + workspace, + path: args.path, + requestBody: submitted, + // As the flow's own run page does: the form fills the main input schema, and a + // preprocessor would take these arguments for a webhook body and hand the flow + // its own output instead. + skipPreprocessor: true + }) + }, + ctx + ) +} + async function testRunFlowByPath( args: z.infer, ctx: WriteDraftCtx From 0d767d00fb340b6684807ec2c6892c648e940e47 Mon Sep 17 00:00:00 2001 From: Diego Imbert <70353967+diegoimbert@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:51:00 +0200 Subject: [PATCH 03/12] refactor: make the acting workspace and user explicit in the entity editors (#11031) * refactor: make the acting workspace and user explicit in the entity editors Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY * fix: resolve the acting user in new-item mode and for the navigation workspace Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY * fix: discard acting-user lookups that no longer describe the acting workspace Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY * refactor: own the acting-user resolution in one composable Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY * fix: key the acting-user cache by a Map and re-ask after a failed lookup Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY * fix: re-ask a failed acting-user lookup when an editor opens a new session Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY * fix: forget a failed acting-user lookup when its workspace stops being the acting one Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY * fix: drop a stale acting-user refusal on arrival rather than on departure Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY * fix: let the navigation user answer for the navigation workspace unconditionally Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY * docs: mark prototype-key workspace ids as unsupported by the entity editors Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY --------- Co-authored-by: Claude Opus 5 (1M context) --- frontend/src/lib/actingUser.svelte.ts | 74 ++++++++++ frontend/src/lib/components/Path.svelte | 34 +++-- .../src/lib/components/ResourceEditor.svelte | 128 +++++++++--------- .../components/ResourceEditorDrawer.svelte | 23 ++-- .../src/lib/components/ResourceForm.svelte | 19 ++- .../src/lib/components/VariableEditor.svelte | 54 ++++---- .../src/lib/components/VariableForm.svelte | 17 ++- .../schedules/ScheduleEditorInner.svelte | 29 +++- .../(root)/(logged)/resources/+page.svelte | 1 + .../(root)/(logged)/variables/+page.svelte | 6 +- 10 files changed, 261 insertions(+), 124 deletions(-) create mode 100644 frontend/src/lib/actingUser.svelte.ts diff --git a/frontend/src/lib/actingUser.svelte.ts b/frontend/src/lib/actingUser.svelte.ts new file mode 100644 index 0000000000..30da65ff83 --- /dev/null +++ b/frontend/src/lib/actingUser.svelte.ts @@ -0,0 +1,74 @@ +import { untrack } from 'svelte' +import { fromStore } from 'svelte/store' +import { SvelteMap } from 'svelte/reactivity' +import { userStore, workspaceStore, type UserExt } from '$lib/stores' +import { getWorkspaceRole, type RoleLookup } from '$lib/user' + +/** + * The user acting in a workspace that is not necessarily the one the top nav points at — an AI + * session or a workspace-specific variant acts on a workspace the nav deliberately is not on. + * + * `$userStore` answers for the navigation workspace at no cost, exactly as every permission check + * in the app did before this hook existed — including when it holds nobody, which reads as unknown + * and refuses. Every other workspace is looked up, and an unresolved user there is `undefined`: it + * must never fall back to the navigation user, whose rights belong to another workspace. + * `canWrite`/`isOwner` refuse for an unknown user, which is the only safe answer. A caller that + * must not render that refusal as a denial asks `resolved` first. + */ +export function useActingUser(workspace: () => string | undefined) { + const navWorkspace = fromStore(workspaceStore) + const navUser = fromStore(userStore) + const looked = new SvelteMap() + // The workspace this effect last acted on, so arriving at one is distinguishable from the + // effect re-running while already there. + let asking: string | undefined + + $effect(() => { + const ws = workspace() + if (asking !== ws) { + asking = ws + // Dropped on the way *in*, not on the way out: a lookup that fails after the acting + // workspace has already moved on has no entry to clear at the moment it is left, so + // clearing it there would keep a refusal that no attempt is behind any more. + if (ws && untrack(() => looked.get(ws)?.kind) === 'lookup_failed') looked.delete(ws) + } + if (!ws || ws === navWorkspace.current) return + // Any settled answer stops the asking, a failure included — otherwise recording one + // would re-enter this effect and loop. + if (looked.has(ws)) return + untrack(() => { + // Memoized process-wide, so two components pointed at the same workspace share one + // request rather than each issuing their own. + getWorkspaceRole(ws).then((lookup) => looked.set(ws, lookup)) + }) + }) + + function userIn(ws: string | undefined): UserExt | undefined { + if (!ws) return undefined + if (ws === navWorkspace.current) return navUser.current + const lookup = looked.get(ws) + return lookup?.kind === 'resolved' ? lookup.user : undefined + } + + return { + /** The acting user in `ws`, or `undefined` when it is not known. Only workspaces this + * hook has been pointed at are looked up; the rest read as unknown. */ + in: userIn, + /** Whether `ws` has an answer at all — a user, or a lookup that came back without one. + * The navigation workspace always has one: `$userStore`, "nobody" included. */ + resolved: (ws: string | undefined): boolean => + !!ws && (ws === navWorkspace.current || looked.has(ws)), + get current(): UserExt | undefined { + return userIn(workspace()) + }, + /** Drop the lookups that came back empty so they are asked again. Arriving at a + * workspace already does this; a long-lived editor must call this too when it starts a + * fresh session on the workspace it is already on, or a `whoami` that happened to fail + * pins it to "unknown user" for as long as it stays there. */ + forgetFailures(): void { + for (const [ws, lookup] of looked) { + if (lookup.kind === 'lookup_failed') looked.delete(ws) + } + } + } +} diff --git a/frontend/src/lib/components/Path.svelte b/frontend/src/lib/components/Path.svelte index 37eef75bf5..b45d980246 100644 --- a/frontend/src/lib/components/Path.svelte +++ b/frontend/src/lib/components/Path.svelte @@ -26,7 +26,7 @@ AzureTriggerService, EmailTriggerService } from '$lib/gen' - import { superadmin, userStore, workspaceStore } from '$lib/stores' + import { superadmin, userStore, workspaceStore, type UserExt } from '$lib/stores' import { createEventDispatcher, getContext, untrack } from 'svelte' import { writable } from 'svelte/store' import { Alert, Button } from './common' @@ -85,6 +85,11 @@ * workspace when the editor operates on a workspace other than the one the * top nav points at (see the sessions preview / dev-workspace flows). */ workspaceOverride?: string + /** The user acting in `workspaceOverride`, for the owner suggestion and the folder + * write flags. Omit it to stand in the navigation `$userStore`, who is a member of + * the navigation workspace only; pass `null` for "not known (yet)", which that user + * must not answer for either. */ + actingUser?: UserExt | null /** One path that does not count as taken, for a caller creating something that may * already have written there itself — a setup flow correcting its own failed attempt. * Every other existing path is still refused. */ @@ -110,11 +115,16 @@ size = 'md', drawerOffset = 0, workspaceOverride = undefined, + actingUser = undefined, allowedExistingPath = undefined, warnOnRename = true }: Props = $props() let ws = $derived(workspaceOverride ?? $workspaceStore) + // Sole place this component falls back to the ambient user, and only for a caller that + // passed none; everything below reads `user`, so a caller acting on another workspace is + // never mixed with the navigation user's memberships. + let user = $derived(actingUser === undefined ? $userStore : (actingUser ?? undefined)) $effect.pre(() => { if (path == undefined) { @@ -169,17 +179,17 @@ export async function reset() { if (path == '' || path == 'u//' || path?.startsWith('tmp/') || path?.startsWith('hub/')) { - if ($lastMetaUsed == undefined || $lastMetaUsed.owner != $userStore?.username) { + if ($lastMetaUsed == undefined || $lastMetaUsed.owner != user?.username) { meta = { ownerKind: hideUser ? 'folder' : 'user', name: fullNamePlaceholder ?? random_adj() + '_' + namePlaceholder, owner: '' } if (!hideUser) { - if ($userStore?.username?.includes('@')) { - meta.owner = $userStore!.username.split('@')[0].replace(/[^a-zA-Z0-9_]/g, '') + if (user?.username?.includes('@')) { + meta.owner = user!.username.split('@')[0].replace(/[^a-zA-Z0-9_]/g, '') } else { - meta.owner = $userStore!.username! + meta.owner = user!.username! } } } else { @@ -229,9 +239,9 @@ .map((x) => ({ name: x, write: - $userStore?.folders?.includes(x) == true || - ($userStore?.is_admin ?? false) || - ($userStore?.is_super_admin ?? false) + user?.folders?.includes(x) == true || + (user?.is_admin ?? false) || + (user?.is_super_admin ?? false) })) ) } @@ -423,7 +433,7 @@ }) }) $effect.pre(() => { - if (ws && $userStore) { + if (ws && user) { untrack(() => { loadFolders() initPath() @@ -506,7 +516,7 @@ } else { // 'group' is unreachable here (Select only offers user/folder) // but validateName still accepts it for forward-compat. - meta.owner = $userStore?.username?.split('@')[0] ?? '' + meta.owner = user?.username?.split('@')[0] ?? '' } } } @@ -520,7 +530,7 @@
{#if meta.ownerKind === 'user'} {@const userOwnerDisabled = - disabled || !($superadmin || ($userStore?.is_admin ?? false)) || disableEditing} + disabled || !($superadmin || (user?.is_admin ?? false)) || disableEditing}