diff --git a/AGENTS.md b/AGENTS.md index 8a63c9828a..a315f1a67c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,8 +31,9 @@ Open-source platform for internal tools, workflows, API integrations, background `cargo run`; a normal build cannot start one at all. - **Enterprise**: `docs/enterprise.md` — EE file conventions and PR workflow - **Auth surface**: `docs/auth-surface.md` — credential precedence, session/cache invalidation - scope, how OAuth login matches `login_type`, and that every superadmin route refuses `$WM_TOKEN`. - Read before designing anything that creates users, tokens or sessions. + scope, which token labels email their owner at expiry, how OAuth login matches `login_type`, and + that every superadmin route refuses `$WM_TOKEN`. Read before designing anything that creates + users, tokens or sessions. - **Product telemetry**: `docs/feature-telemetry.md` — when to instrument a new feature with `feature_usage`, and the four-step recipe. An unregistered `(feature, kind)` pair is dropped silently, so frontend-only instrumentation records nothing. diff --git a/CONTEXT.md b/CONTEXT.md index 64aa1b93d8..fbc2592cc9 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -37,6 +37,25 @@ _Avoid_: argument field, param Any other place a property can be picked into: the loop iterator, skip and early-stop predicates, the retry condition, a branch predicate, timeout. Its prop picker opens in a popover from the connect button rather than taking a pane. _Avoid_: JS field, code input +### Flow chat + +**Conversation**: +One thread of messages against one chat-enabled flow, with its own agent memory. A flow has +many; the chat shows one at a time. +_Avoid_: thread, session (that names an AI session, a different thing), chat (that names the surface) + +**Turn**: +One question and the answer to it: the run the question started, the handle that stops it, +and the rows it is writing. At most one per conversation, and the chat is held for its whole +length — from the moment the question takes the chat, before it has a job, until it is ended. +_Avoid_: request, exchange, message round + +**Transcript**: +The rows a conversation's chat holds. Not the conversation: it is the newest page plus +whatever older pages the reader has scrolled back through, so a question it cannot answer +from what it holds is one to ask the server rather than to guess at. +_Avoid_: history, messages (too easily read as "all of them") + ### Permissions **Member**: diff --git a/ai_evals/adapters/frontend/mockBackend.ts b/ai_evals/adapters/frontend/mockBackend.ts index 6d80f19197..8c1ca431b9 100644 --- a/ai_evals/adapters/frontend/mockBackend.ts +++ b/ai_evals/adapters/frontend/mockBackend.ts @@ -92,7 +92,7 @@ export interface BenchmarkWorkspaceResource { } export interface BenchmarkWorkspaceJob { - /** Stable id so a case prompt can reference a specific run (e.g. for get_job_logs). */ + /** Stable id so a case prompt can reference a specific run (e.g. for get_run). */ id?: string jobKind?: CompletedJob['job_kind'] scriptPath?: string @@ -100,6 +100,8 @@ export interface BenchmarkWorkspaceJob { label?: string success?: boolean logs?: string + args?: Record + result?: unknown } export interface BenchmarkWorkspaceRunnables { @@ -156,7 +158,7 @@ export function registerBenchmarkWorkspaceRunnables( ...runnables, datatables: runnables.datatables ? structuredClone(runnables.datatables) : undefined }) - // Seed any fixture jobs so list_runs / get_job_logs have data to return. + // Seed any fixture jobs so list_runs / get_run have data to return. for (const seed of runnables.jobs ?? []) { createBenchmarkCompletedJob({ workspace, @@ -166,7 +168,9 @@ export function registerBenchmarkWorkspaceRunnables( scriptPath: seed.scriptPath, createdBy: seed.createdBy, label: seed.label, - logs: seed.logs + logs: seed.logs, + args: seed.args, + result: seed.result }) } } @@ -481,6 +485,33 @@ export function getBenchmarkJobLogs(workspace: string, jobId: string): string { return job.logs ?? '' } +/** + * Mirror `JobService.getFlowAllResults`, which get_run calls for the execution + * tree. Fixture jobs are single runs with no steps, so only the root entry. + */ +export function getBenchmarkFlowAllResults(workspace: string, jobId: string) { + const job = getBenchmarkCompletedJob(workspace, jobId) + if (!job) { + throw new Error(`Job "${jobId}" not found in benchmark workspace`) + } + return { + entries: [ + { + job_id: jobId, + label: 'Flow', + kind: job.job_kind ?? 'script', + depth: 0, + sibling_index: 1, + sibling_count: 1, + status: job.success ? 'success' : 'failure', + success: job.success + } + ], + truncated: false, + scope_filtered: false + } +} + // ============= Drafts (per-user, DB-backed in production) ============= /** diff --git a/ai_evals/adapters/frontend/vitestAdapter.test.ts b/ai_evals/adapters/frontend/vitestAdapter.test.ts index e5b275b86e..f240d0ee36 100644 --- a/ai_evals/adapters/frontend/vitestAdapter.test.ts +++ b/ai_evals/adapters/frontend/vitestAdapter.test.ts @@ -62,6 +62,7 @@ vi.mock('$lib/gen', async () => { getBenchmarkDatatableSchema, getBenchmarkDraftForUser, getBenchmarkFlowByPath, + getBenchmarkFlowAllResults, getBenchmarkJobLogs, getBenchmarkOwnDraft, getBenchmarkScriptByHash, @@ -325,7 +326,11 @@ vi.mock('$lib/gen', async () => { getJobLogs: async (data: { workspace: string; id: string }) => hasBenchmarkWorkspace(data.workspace) ? getBenchmarkJobLogs(data.workspace, data.id) - : actual.JobService.getJobLogs(data) + : actual.JobService.getJobLogs(data), + getFlowAllResults: async (data: { workspace: string; id: string }) => + hasBenchmarkWorkspace(data.workspace) + ? getBenchmarkFlowAllResults(data.workspace, data.id) + : actual.JobService.getFlowAllResults(data) }), WorkspaceService: wrapService(actual.WorkspaceService, { getCopilotInfo: async (data: { workspace: string }) => diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml index 71693156a9..951d316524 100644 --- a/ai_evals/cases/global.yaml +++ b/ai_evals/cases/global.yaml @@ -889,13 +889,13 @@ draftCountExactly: 0 toolExpect: requiredToolsUsed: - - get_job_logs + - get_run forbiddenToolsUsed: - deploy_workspace_item - delete_workspace_item - write_script toolCallArgs: - - tool: get_job_logs + - tool: get_run field: id stringIncludesAnyOf: - 01920000-0000-7000-8000-0000000000f1 @@ -906,6 +906,34 @@ - fetches the logs for the requested job id - explains the failure from the returned logs (connection refused to the upstream API) +- id: global-run-args-and-result + prompt: |- + What was the run 01920000-0000-7000-8000-0000000000f2 called with, and what did it return? + initial: ai_evals/fixtures/frontend/global/initial/jobs_seed.json + runtime: + maxTurns: 6 + validate: + draftCountExactly: 0 + toolExpect: + requiredToolsUsed: + - get_run + forbiddenToolsUsed: + - test_run_script + - run_script + - deploy_workspace_item + toolCallArgs: + - tool: get_run + field: id + stringIncludesAnyOf: + - 01920000-0000-7000-8000-0000000000f2 + # Read-only, so no draft for the global judge to score — validated on tool use + # and the deterministic argument check, like the neighbouring run cases. + skipJudge: true + judgeChecklist: + - reports the arguments the run was called with (region emea, 12 recipients) + - reports what the run returned (12 sent, 3 skipped) + - does not start a new run to find out + # --- Page navigation (open_page) --- # The assistant should take the user to a Windmill page (Runs/Schedules) with the # right filters via open_page, rather than describing where to click or dumping the diff --git a/ai_evals/fixtures/frontend/global/initial/jobs_seed.json b/ai_evals/fixtures/frontend/global/initial/jobs_seed.json index b075d6df26..4c0f923553 100644 --- a/ai_evals/fixtures/frontend/global/initial/jobs_seed.json +++ b/ai_evals/fixtures/frontend/global/initial/jobs_seed.json @@ -15,6 +15,8 @@ "jobKind": "script", "createdBy": "bob", "success": true, + "args": { "region": "emea", "dry_run": false, "recipients": 12 }, + "result": { "sent": 12, "skipped": 3, "digest_url": "https://reports.example.com/d/2026-06-09" }, "logs": "Generating daily digest...\nDigest emailed to 12 recipients\nDone in 1.2s" }, { diff --git a/backend/.sqlx/query-07a005f0f9e80a156cd2a5a0ae39a1fabeaa167818206a25abfe31d5582f942a.json b/backend/.sqlx/query-07a005f0f9e80a156cd2a5a0ae39a1fabeaa167818206a25abfe31d5582f942a.json new file mode 100644 index 0000000000..635ec2b94d --- /dev/null +++ b/backend/.sqlx/query-07a005f0f9e80a156cd2a5a0ae39a1fabeaa167818206a25abfe31d5582f942a.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM flow_conversation c\n WHERE c.id = ANY($1)\n AND c.workspace_id = $2\n AND NOT EXISTS (\n SELECT 1 FROM flow_conversation_message m WHERE m.conversation_id = c.id\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "UuidArray", + "Text" + ] + }, + "nullable": [] + }, + "hash": "07a005f0f9e80a156cd2a5a0ae39a1fabeaa167818206a25abfe31d5582f942a" +} diff --git a/backend/.sqlx/query-462d2b2822b185a6f51fafcfa957cb3b31ee6b69abae79a214dddba0dee4425c.json b/backend/.sqlx/query-462d2b2822b185a6f51fafcfa957cb3b31ee6b69abae79a214dddba0dee4425c.json deleted file mode 100644 index 0c4d90b073..0000000000 --- a/backend/.sqlx/query-462d2b2822b185a6f51fafcfa957cb3b31ee6b69abae79a214dddba0dee4425c.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "DELETE FROM flow_conversation_message m\n USING flow_conversation c\n WHERE m.conversation_id = c.id AND c.workspace_id = $1 AND m.job_id = ANY($2)", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Text", - "UuidArray" - ] - }, - "nullable": [] - }, - "hash": "462d2b2822b185a6f51fafcfa957cb3b31ee6b69abae79a214dddba0dee4425c" -} diff --git a/backend/.sqlx/query-4f52bf546579f26a1d22c239b8b0054b753cfbeb5dad1e8120fd5e8a672d50ef.json b/backend/.sqlx/query-4f52bf546579f26a1d22c239b8b0054b753cfbeb5dad1e8120fd5e8a672d50ef.json new file mode 100644 index 0000000000..a5695f75fd --- /dev/null +++ b/backend/.sqlx/query-4f52bf546579f26a1d22c239b8b0054b753cfbeb5dad1e8120fd5e8a672d50ef.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT id FROM flow_conversation WHERE id = ANY($1) AND workspace_id = $2 ORDER BY id FOR UPDATE", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Uuid" + } + ], + "parameters": { + "Left": [ + "UuidArray", + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "4f52bf546579f26a1d22c239b8b0054b753cfbeb5dad1e8120fd5e8a672d50ef" +} diff --git a/backend/.sqlx/query-69bfbe9b39414b724488532cc3b3659915d9fcb3d58f16532aaffe06c44ec976.json b/backend/.sqlx/query-69bfbe9b39414b724488532cc3b3659915d9fcb3d58f16532aaffe06c44ec976.json new file mode 100644 index 0000000000..52105ca605 --- /dev/null +++ b/backend/.sqlx/query-69bfbe9b39414b724488532cc3b3659915d9fcb3d58f16532aaffe06c44ec976.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM flow_conversation_message m\n USING flow_conversation c\n WHERE m.conversation_id = c.id AND c.workspace_id = $1 AND m.job_id = ANY($2)\n RETURNING m.conversation_id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "conversation_id", + "type_info": "Uuid" + } + ], + "parameters": { + "Left": [ + "Text", + "UuidArray" + ] + }, + "nullable": [ + false + ] + }, + "hash": "69bfbe9b39414b724488532cc3b3659915d9fcb3d58f16532aaffe06c44ec976" +} diff --git a/backend/.sqlx/query-6f32c1feed096ff706ae359ad6a3ca33b3f82ca38289dfa4a69aa95041027d57.json b/backend/.sqlx/query-6f32c1feed096ff706ae359ad6a3ca33b3f82ca38289dfa4a69aa95041027d57.json new file mode 100644 index 0000000000..0e92e3aa99 --- /dev/null +++ b/backend/.sqlx/query-6f32c1feed096ff706ae359ad6a3ca33b3f82ca38289dfa4a69aa95041027d57.json @@ -0,0 +1,59 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT id, workspace_id, flow_path, title, created_at, updated_at, created_by\n FROM flow_conversation\n WHERE id = $1 AND workspace_id = $2\n FOR UPDATE", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Uuid" + }, + { + "ordinal": 1, + "name": "workspace_id", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "flow_path", + "type_info": "Varchar" + }, + { + "ordinal": 3, + "name": "title", + "type_info": "Varchar" + }, + { + "ordinal": 4, + "name": "created_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 5, + "name": "updated_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 6, + "name": "created_by", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Uuid", + "Text" + ] + }, + "nullable": [ + false, + false, + false, + true, + false, + false, + false + ] + }, + "hash": "6f32c1feed096ff706ae359ad6a3ca33b3f82ca38289dfa4a69aa95041027d57" +} diff --git a/backend/.sqlx/query-89ea81b765550cf665e30533efc9672f8c98d2579fc72c07752361cc5fd683dc.json b/backend/.sqlx/query-89ea81b765550cf665e30533efc9672f8c98d2579fc72c07752361cc5fd683dc.json new file mode 100644 index 0000000000..7a7110030c --- /dev/null +++ b/backend/.sqlx/query-89ea81b765550cf665e30533efc9672f8c98d2579fc72c07752361cc5fd683dc.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM ai_agent_memory a\n USING flow_conversation c\n WHERE c.id = ANY($1)\n AND c.workspace_id = $2\n AND a.conversation_id = c.id\n AND a.workspace_id = c.workspace_id\n AND NOT EXISTS (\n SELECT 1 FROM flow_conversation_message m WHERE m.conversation_id = c.id\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "UuidArray", + "Text" + ] + }, + "nullable": [] + }, + "hash": "89ea81b765550cf665e30533efc9672f8c98d2579fc72c07752361cc5fd683dc" +} diff --git a/backend/.sqlx/query-90b910da8d00a7c7bcf29c167e38e44eb1c0062a8241dc8fe0ed3dd95b65f89a.json b/backend/.sqlx/query-90b910da8d00a7c7bcf29c167e38e44eb1c0062a8241dc8fe0ed3dd95b65f89a.json new file mode 100644 index 0000000000..1555f3683d --- /dev/null +++ b/backend/.sqlx/query-90b910da8d00a7c7bcf29c167e38e44eb1c0062a8241dc8fe0ed3dd95b65f89a.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM flow_conversation c\n WHERE c.id = ANY($1)\n AND NOT EXISTS (\n SELECT 1 FROM flow_conversation_message m WHERE m.conversation_id = c.id\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "UuidArray" + ] + }, + "nullable": [] + }, + "hash": "90b910da8d00a7c7bcf29c167e38e44eb1c0062a8241dc8fe0ed3dd95b65f89a" +} diff --git a/backend/.sqlx/query-967f52005f4a044b3a2e9f02ceadf90dad5246681bde6caaa633b85a5e8b2352.json b/backend/.sqlx/query-967f52005f4a044b3a2e9f02ceadf90dad5246681bde6caaa633b85a5e8b2352.json new file mode 100644 index 0000000000..5161f716ff --- /dev/null +++ b/backend/.sqlx/query-967f52005f4a044b3a2e9f02ceadf90dad5246681bde6caaa633b85a5e8b2352.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM flow_conversation_message WHERE job_id = ANY($1) RETURNING conversation_id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "conversation_id", + "type_info": "Uuid" + } + ], + "parameters": { + "Left": [ + "UuidArray" + ] + }, + "nullable": [ + false + ] + }, + "hash": "967f52005f4a044b3a2e9f02ceadf90dad5246681bde6caaa633b85a5e8b2352" +} diff --git a/backend/.sqlx/query-a1b23f3e62c6433d95cdac58215741a51ca0ce66bf1c674097705cf2e1b72eff.json b/backend/.sqlx/query-a1b23f3e62c6433d95cdac58215741a51ca0ce66bf1c674097705cf2e1b72eff.json new file mode 100644 index 0000000000..d8fdd5140b --- /dev/null +++ b/backend/.sqlx/query-a1b23f3e62c6433d95cdac58215741a51ca0ce66bf1c674097705cf2e1b72eff.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM ai_agent_memory a\n USING flow_conversation c\n WHERE c.id = ANY($1)\n AND a.conversation_id = c.id\n AND a.workspace_id = c.workspace_id\n AND NOT EXISTS (\n SELECT 1 FROM flow_conversation_message m WHERE m.conversation_id = c.id\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "UuidArray" + ] + }, + "nullable": [] + }, + "hash": "a1b23f3e62c6433d95cdac58215741a51ca0ce66bf1c674097705cf2e1b72eff" +} diff --git a/backend/.sqlx/query-bfdd60b42e32bd81e2d20b327462893147b4e5ff078531de36147d908132d636.json b/backend/.sqlx/query-bfdd60b42e32bd81e2d20b327462893147b4e5ff078531de36147d908132d636.json deleted file mode 100644 index f9fbc7a58b..0000000000 --- a/backend/.sqlx/query-bfdd60b42e32bd81e2d20b327462893147b4e5ff078531de36147d908132d636.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "DELETE FROM flow_conversation_message WHERE job_id = ANY($1)", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "UuidArray" - ] - }, - "nullable": [] - }, - "hash": "bfdd60b42e32bd81e2d20b327462893147b4e5ff078531de36147d908132d636" -} diff --git a/backend/.sqlx/query-6bd23a98838e3eec309e6b696edc776bd56fc9dae1238b3272557d1562400dbe.json b/backend/.sqlx/query-c1e3ed3ecc3bcb98f60ba8196d33fee4a74f61b061e5025ecb75882208b3ba8f.json similarity index 84% rename from backend/.sqlx/query-6bd23a98838e3eec309e6b696edc776bd56fc9dae1238b3272557d1562400dbe.json rename to backend/.sqlx/query-c1e3ed3ecc3bcb98f60ba8196d33fee4a74f61b061e5025ecb75882208b3ba8f.json index d7f5fc45d4..50ba9d2897 100644 --- a/backend/.sqlx/query-6bd23a98838e3eec309e6b696edc776bd56fc9dae1238b3272557d1562400dbe.json +++ b/backend/.sqlx/query-c1e3ed3ecc3bcb98f60ba8196d33fee4a74f61b061e5025ecb75882208b3ba8f.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "INSERT INTO flow_conversation (id, workspace_id, flow_path, created_by, title)\n VALUES ($1, $2, $3, $4, $5)\n RETURNING id, workspace_id, flow_path, title, created_at, updated_at, created_by", + "query": "INSERT INTO flow_conversation (id, workspace_id, flow_path, created_by, title)\n VALUES ($1, $2, $3, $4, $5)\n ON CONFLICT (id) DO NOTHING\n RETURNING id, workspace_id, flow_path, title, created_at, updated_at, created_by", "describe": { "columns": [ { @@ -58,5 +58,5 @@ false ] }, - "hash": "6bd23a98838e3eec309e6b696edc776bd56fc9dae1238b3272557d1562400dbe" + "hash": "c1e3ed3ecc3bcb98f60ba8196d33fee4a74f61b061e5025ecb75882208b3ba8f" } diff --git a/backend/.sqlx/query-31ed2fb85c0c726e3cf6392be2a73c62bae004b2842a4828c6807232570f83a1.json b/backend/.sqlx/query-d631a26e5531589ff37e677f91a4f1d9f850e3e46c17130dd580426cda7a9f65.json similarity index 63% rename from backend/.sqlx/query-31ed2fb85c0c726e3cf6392be2a73c62bae004b2842a4828c6807232570f83a1.json rename to backend/.sqlx/query-d631a26e5531589ff37e677f91a4f1d9f850e3e46c17130dd580426cda7a9f65.json index 556bd7c317..8cef9fc8aa 100644 --- a/backend/.sqlx/query-31ed2fb85c0c726e3cf6392be2a73c62bae004b2842a4828c6807232570f83a1.json +++ b/backend/.sqlx/query-d631a26e5531589ff37e677f91a4f1d9f850e3e46c17130dd580426cda7a9f65.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "UPDATE token SET label = $1\n WHERE email = $2 AND token_prefix = $3\n AND (label IS NULL OR (\n label <> 'session'\n AND label <> 'guest_session'\n AND lower(label) NOT LIKE 'ephemeral%'\n AND label <> 'debugger-token'\n AND label NOT LIKE 'mcp-oauth-%'\n ))\n RETURNING token_prefix", + "query": "UPDATE token SET label = $1\n WHERE email = $2 AND token_prefix = $3\n AND (label IS NULL OR (\n label <> 'session'\n AND label <> 'guest_session'\n AND lower(label) NOT LIKE 'ephemeral%'\n AND label <> 'debugger-token'\n AND label NOT LIKE 'mcp-oauth-%'\n AND NOT starts_with(label, 'embed_app:')\n AND NOT starts_with(label, 'sdk_app:')\n AND NOT starts_with(label, 'impersonation:')\n ))\n RETURNING token_prefix", "describe": { "columns": [ { @@ -20,5 +20,5 @@ false ] }, - "hash": "31ed2fb85c0c726e3cf6392be2a73c62bae004b2842a4828c6807232570f83a1" + "hash": "d631a26e5531589ff37e677f91a4f1d9f850e3e46c17130dd580426cda7a9f65" } diff --git a/backend/.sqlx/query-ec295b3890a0018475ec0a3774c7a30d71a5689efe72daf58bd1e8f6cf90c410.json b/backend/.sqlx/query-ec295b3890a0018475ec0a3774c7a30d71a5689efe72daf58bd1e8f6cf90c410.json new file mode 100644 index 0000000000..1e35bd15ac --- /dev/null +++ b/backend/.sqlx/query-ec295b3890a0018475ec0a3774c7a30d71a5689efe72daf58bd1e8f6cf90c410.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT id FROM flow_conversation WHERE id = ANY($1) ORDER BY id FOR UPDATE", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Uuid" + } + ], + "parameters": { + "Left": [ + "UuidArray" + ] + }, + "nullable": [ + false + ] + }, + "hash": "ec295b3890a0018475ec0a3774c7a30d71a5689efe72daf58bd1e8f6cf90c410" +} diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 082682b52a..4484086177 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -119cefe26b3d78668cefe40be14c64eae529b35d +16ef314b575fde3db031d598f6bbcb462d8903fc diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index dc3b9b6e8b..e250302209 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -4712,17 +4712,11 @@ const GIT_AUTO_PULL_LOCK_ID: i64 = 737_483_921; /// Poll every git-sync repository with auto-pull enabled and enqueue a pull when /// the tracked branch has new commits (repo → Windmill direction). /// -/// Runs on a single replica at a time (advisory lock) and only on -/// Enterprise-licensed instances. Detection is `git ls-remote`; GitHub-App -/// repositories are skipped here and sync via webhooks instead (phase 2). +/// Runs on a single replica at a time (advisory lock). Detection is +/// `git ls-remote`; GitHub-App repositories are skipped here and sync via +/// webhooks instead (phase 2). #[cfg(feature = "private")] pub async fn poll_git_auto_pull(db: &Pool) { - use windmill_common::ee_oss::{get_license_plan, LicensePlan}; - - if !matches!(get_license_plan().await, LicensePlan::Enterprise) { - return; - } - let mut lock_conn = match db.acquire().await { Ok(c) => c, Err(e) => { @@ -4792,12 +4786,6 @@ const GIT_CREDENTIAL_LOCK_ID: i64 = 737_483_923; /// sync down on its expiry date. #[cfg(all(feature = "enterprise", feature = "private"))] async fn maintain_git_credentials(db: &Pool) { - use windmill_common::ee_oss::{get_license_plan, LicensePlan}; - - if !matches!(get_license_plan().await, LicensePlan::Enterprise) { - return; - } - // Transaction-scoped advisory lock, as for the schedule reconcile above: a // session lock on a pooled connection would ride back into the pool still // held if the sweep died before unlocking, and wedge the pass on every diff --git a/backend/tests/v2_job_delete_orphans.rs b/backend/tests/v2_job_delete_orphans.rs index 95cd1673b6..ded760b8ff 100644 --- a/backend/tests/v2_job_delete_orphans.rs +++ b/backend/tests/v2_job_delete_orphans.rs @@ -37,7 +37,7 @@ async fn seed_side_rows(db: &Pool, ws: &str, job_id: Uuid) -> anyhow:: .bind(ws) .execute(db) .await?; - // created_seq is assigned by a trigger; inserting a value is rejected. + // created_seq is an identity column; supplying a value is rejected. sqlx::query( "INSERT INTO flow_conversation_message (conversation_id, message_type, content, job_id) VALUES ($1, 'assistant', 'hi', $2)", @@ -121,6 +121,184 @@ async fn test_delete_jobs_removes_side_rows(db: Pool) -> anyhow::Resul Ok(()) } +/// (conversation rows, agent-memory rows) for one conversation. +async fn conversation_and_memory_counts( + db: &Pool, + conversation_id: Uuid, +) -> anyhow::Result<(i64, i64)> { + Ok(( + count( + db, + "SELECT count(*) FROM flow_conversation WHERE id = $1", + conversation_id, + ) + .await?, + count( + db, + "SELECT count(*) FROM ai_agent_memory WHERE conversation_id = $1", + conversation_id, + ) + .await?, + )) +} + +/// A conversation outlives the jobs behind its messages until the last one goes: only then +/// are the row and the agent's memory for it left with nothing, and only then are they +/// deleted. Both halves matter — the surviving half is what a single data-modifying CTE +/// would break, since its emptiness check would read the snapshot from before the delete. +#[sqlx::test(fixtures("base"))] +async fn test_delete_jobs_removes_a_conversation_once_its_last_message_goes( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + + let first_job = Uuid::new_v4(); + let second_job = Uuid::new_v4(); + insert_job(&db, WS, first_job).await?; + insert_job(&db, WS, second_job).await?; + + let conv_id = Uuid::new_v4(); + sqlx::query( + "INSERT INTO flow_conversation (id, workspace_id, flow_path, created_by) + VALUES ($1, $2, 'f/flow', 'test-user')", + ) + .bind(conv_id) + .bind(WS) + .execute(&db) + .await?; + for job_id in [first_job, second_job] { + sqlx::query( + "INSERT INTO flow_conversation_message (conversation_id, message_type, content, job_id) + VALUES ($1, 'assistant', 'hi', $2)", + ) + .bind(conv_id) + .bind(job_id) + .execute(&db) + .await?; + } + sqlx::query( + "INSERT INTO ai_agent_memory (workspace_id, conversation_id, step_id, messages) + VALUES ($1, $2, 'a', '[]'::jsonb)", + ) + .bind(WS) + .bind(conv_id) + .execute(&db) + .await?; + + let mut conn = db.acquire().await?; + windmill_common::jobs::delete_jobs(&mut conn, &[first_job]).await?; + drop(conn); + assert_eq!( + conversation_and_memory_counts(&db, conv_id).await?, + (1, 1), + "a conversation with a message left must survive, memory included" + ); + + let mut conn = db.acquire().await?; + windmill_common::jobs::delete_jobs(&mut conn, &[second_job]).await?; + drop(conn); + assert_eq!( + conversation_and_memory_counts(&db, conv_id).await?, + (0, 0), + "the last message going should take the conversation and its memory" + ); + Ok(()) +} + +/// Turns that start while retention is collecting their conversation must land, not fail: +/// the conversation lookup locks the row, so each turn waits for the collector's commit, +/// finds the conversation gone, and creates it again — the first insert wins and the other +/// reads its row. Without the lock a turn's message insert is what waits, on the parent +/// row's key lock, and fails its FK check afterwards. +#[sqlx::test(fixtures("base"))] +async fn test_new_turns_wait_for_conversation_cleanup_and_recreate( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + + let old_job = Uuid::new_v4(); + let new_jobs = [Uuid::new_v4(), Uuid::new_v4()]; + insert_job(&db, WS, old_job).await?; + for job in new_jobs { + insert_job(&db, WS, job).await?; + } + + let conv_id = Uuid::new_v4(); + sqlx::query( + "INSERT INTO flow_conversation (id, workspace_id, flow_path, created_by) + VALUES ($1, $2, 'f/flow', 'test-user')", + ) + .bind(conv_id) + .bind(WS) + .execute(&db) + .await?; + sqlx::query( + "INSERT INTO flow_conversation_message (conversation_id, message_type, content, job_id) + VALUES ($1, 'user', 'hi', $2)", + ) + .bind(conv_id) + .bind(old_job) + .execute(&db) + .await?; + + // The collector holds the conversation row locked and deleted, uncommitted. + let mut cleanup = db.begin().await?; + windmill_common::jobs::delete_jobs(&mut *cleanup, &[old_job]).await?; + + let turns: Vec<_> = new_jobs + .into_iter() + .map(|new_job| { + let db = db.clone(); + tokio::spawn(async move { + let mut tx = db.begin().await?; + windmill_common::flow_conversations::get_or_create_conversation_with_id( + &mut tx, + WS, + "f/flow", + "test-user", + "hi again", + conv_id, + ) + .await?; + windmill_common::flow_conversations::add_message_to_conversation_tx( + &mut tx, + conv_id, + Some(new_job), + "hi again", + windmill_common::flow_conversations::MessageType::User, + None, + true, + ) + .await?; + tx.commit().await?; + anyhow::Ok(()) + }) + }) + .collect(); + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + cleanup.commit().await?; + for turn in turns { + turn.await??; + } + + assert_eq!( + conversation_and_memory_counts(&db, conv_id).await?.0, + 1, + "the turns must have created the conversation again, once" + ); + assert_eq!( + count( + &db, + "SELECT count(*) FROM flow_conversation_message WHERE conversation_id = $1", + conv_id, + ) + .await?, + 2, + "both turns' messages should be there" + ); + Ok(()) +} + #[sqlx::test(fixtures("base"))] async fn test_clear_schedule_removes_side_rows(db: Pool) -> anyhow::Result<()> { initialize_tracing().await; @@ -192,6 +370,74 @@ async fn test_workspace_delete_removes_side_rows(db: Pool) -> anyhow:: Ok(()) } +/// The purge endpoint carries its own copy of the emptied-conversation rule, so it gets the +/// same guard: the conversation and its memory go with the last message, and not before. +#[sqlx::test(fixtures("base"))] +async fn test_jobs_export_delete_removes_a_conversation_once_its_last_message_goes( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + + let first_job = Uuid::new_v4(); + let second_job = Uuid::new_v4(); + insert_job(&db, WS, first_job).await?; + insert_job(&db, WS, second_job).await?; + + let conv_id = Uuid::new_v4(); + sqlx::query( + "INSERT INTO flow_conversation (id, workspace_id, flow_path, created_by) + VALUES ($1, $2, 'f/flow', 'test-user')", + ) + .bind(conv_id) + .bind(WS) + .execute(&db) + .await?; + for job_id in [first_job, second_job] { + sqlx::query( + "INSERT INTO flow_conversation_message (conversation_id, message_type, content, job_id) + VALUES ($1, 'assistant', 'hi', $2)", + ) + .bind(conv_id) + .bind(job_id) + .execute(&db) + .await?; + } + sqlx::query( + "INSERT INTO ai_agent_memory (workspace_id, conversation_id, step_id, messages) + VALUES ($1, $2, 'a', '[]'::jsonb)", + ) + .bind(WS) + .bind(conv_id) + .execute(&db) + .await?; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let purge = |job_id: Uuid| async move { + reqwest::Client::new() + .post(format!("http://localhost:{port}/api/w/{WS}/jobs/delete")) + .header("Authorization", "Bearer SECRET_TOKEN") + .json(&[job_id]) + .send() + .await + }; + + assert!(purge(first_job).await?.status().is_success()); + assert_eq!( + conversation_and_memory_counts(&db, conv_id).await?, + (1, 1), + "a conversation with a message left must survive the purge endpoint too" + ); + + assert!(purge(second_job).await?.status().is_success()); + assert_eq!( + conversation_and_memory_counts(&db, conv_id).await?, + (0, 0), + "the last message going should take the conversation and its memory" + ); + Ok(()) +} + /// The `/jobs/delete` purge endpoint must scope every side-table delete to the path /// workspace. A `test-workspace` admin passing a job id from another workspace must not be /// able to delete that workspace's job or side rows (the side tables no longer cascade, so diff --git a/backend/windmill-api-auth/src/lib.rs b/backend/windmill-api-auth/src/lib.rs index 04513b4c05..0e26c2a8b8 100644 --- a/backend/windmill-api-auth/src/lib.rs +++ b/backend/windmill-api-auth/src/lib.rs @@ -1141,6 +1141,9 @@ impl NewToken { /// [`ensure_scopes_within_caller`] first (internal narrowing mints intentionally /// skip it, since their scopes derive from the action being authorized, not the /// caller's token). +/// +/// A token the system mints for itself with an `expiration` needs a label reserved in +/// `windmill_common::auth::is_user_token`, or its expiry alerts its owner (docs/auth-surface.md). pub async fn create_token_internal( tx: &mut sqlx::PgConnection, db: &DB, diff --git a/backend/windmill-api-integration-tests/tests/datatable_roles.rs b/backend/windmill-api-integration-tests/tests/datatable_roles.rs index d79dd5a3e3..10b273cbc7 100644 --- a/backend/windmill-api-integration-tests/tests/datatable_roles.rs +++ b/backend/windmill-api-integration-tests/tests/datatable_roles.rs @@ -912,6 +912,18 @@ async fn a_stored_name_containing_a_question_mark_resolves_as_itself( resolve("main?dt").await.is_err(), "an unknown parameter was ignored" ); + + sqlx::query( + "UPDATE workspace_settings + SET datatable = jsonb_set(datatable, '{datatables,main?role=analytics}', datatable->'datatables'->'main') + WHERE workspace_id = 'test-workspace'", + ) + .execute(&db) + .await?; + assert!( + resolve("main?role=analytics").await.is_err(), + "a reference naming both a stored data table and a role on another resolved to one of them" + ); Ok(()) } @@ -1073,6 +1085,68 @@ async fn browsing_as_a_role_the_caller_may_not_use_is_refused( Ok(()) } +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn an_alias_saved_elsewhere_waits_for_roles_going_on_for_its_database( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + sqlx::query( + r#"UPDATE workspace_settings SET datatable = jsonb_set(datatable, '{datatables,other}', + '{"database": {"resource_type": "instance", "resource_path": "dt_other"}}') + WHERE workspace_id = 'test-workspace'"#, + ) + .execute(&db) + .await?; + + // Roles going on for `dt_other`, not committed yet: it holds only its own workspace's settings + // row, so an alias saved from another workspace that looked for roles now would miss them. + let enabling = { + let mut tx = db.begin().await?; + windmill_common::datatable_roles::lock_instance_databases_governance( + &mut *tx, + ["dt_other"], + ) + .await?; + sqlx::query( + r#"UPDATE workspace_settings SET datatable = jsonb_set(datatable, + '{datatables,other,permissions}', + '{"default_role": "admin", "roles": {"admin": {"tenants": ["*"]}}}') + WHERE workspace_id = 'test-workspace'"#, + ) + .execute(&mut *tx) + .await?; + tx + }; + + let server = ApiServer::start(db.clone()).await?; + let url = format!( + "http://localhost:{}/api/w/wm-fork-dt/workspaces/edit_datatable_config", + server.addr.port() + ); + let save = tokio::spawn( + authed(client().post(&url), "SECRET_TOKEN") + .json(&json!({ "settings": { "datatables": { + "direct": { "database": { "resource_type": "instance", "resource_path": "dt_other" } } + } } })) + .send(), + ); + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + assert!( + !save.is_finished(), + "an alias was saved while roles were going on for its database" + ); + enabling.commit().await?; + + let resp = save.await??; + let status = resp.status(); + let body = resp.text().await?; + assert!( + status == 400 && body.contains("which a data table under roles uses"), + "the alias reached the database whose roles went on while it waited ({status}): {body}" + ); + Ok(()) +} + #[cfg(not(all(feature = "private", feature = "enterprise")))] const ENTERPRISE_REFUSAL: &str = "Data table roles are a Windmill Enterprise Edition feature"; @@ -1162,22 +1236,25 @@ async fn without_the_enterprise_edition_a_data_table_under_roles_is_refused_a_co assert!(err.to_string().contains(ENTERPRISE_REFUSAL), "{err}"); } - // Not under roles, it resolves as it always has; naming a role on it is refused. + // Not under roles, it resolves as it always has, including when `admin` is named — which every + // migration does; naming any other role on it is refused. sqlx::query( "UPDATE workspace_settings SET datatable = datatable #- '{datatables,main,permissions}' WHERE workspace_id = 'test-workspace'", ) .execute(&db) .await?; - let resolved = get_datatable_resource_from_db( - &db, - "test-workspace", - "main", - None, - DatatableAccess::NoIdentity, - ) - .await?; - assert_eq!(resolved["dbname"], "dt_main", "{resolved}"); + for role in [None, Some("admin")] { + let resolved = get_datatable_resource_from_db( + &db, + "test-workspace", + "main", + role, + DatatableAccess::NoIdentity, + ) + .await?; + assert_eq!(resolved["dbname"], "dt_main", "{resolved}"); + } let err = get_datatable_resource_from_db( &db, "test-workspace", diff --git a/backend/windmill-api-integration-tests/tests/token_label_idor.rs b/backend/windmill-api-integration-tests/tests/token_label_idor.rs index ddec1a481a..e690bf673f 100644 --- a/backend/windmill-api-integration-tests/tests/token_label_idor.rs +++ b/backend/windmill-api-integration-tests/tests/token_label_idor.rs @@ -179,9 +179,11 @@ async fn test_trigger_token_labels_still_creatable(db: Pool) -> anyhow "http-test-user-2-cd34", "email-test-user-2-ef56", "my-ci-token", - // Minted client-side by the editor (every TypeScript editor load) and the debugger. + // Minted client-side by the editor (every TypeScript editor load), the debugger and + // the object-storage "Test from a worker" button. "Ephemeral lsp token", "debugger-token", + "ephemeral-test-connection: s3_bucket", ] { let resp = create_token_with_label(port, "SECRET_TOKEN_2", label).await; assert_eq!( diff --git a/backend/windmill-api-jobs/src/execution.rs b/backend/windmill-api-jobs/src/execution.rs index 310e44ae1f..c15068583d 100644 --- a/backend/windmill-api-jobs/src/execution.rs +++ b/backend/windmill-api-jobs/src/execution.rs @@ -668,10 +668,16 @@ pub async fn handle_chat_conversation_messages( flow_path: &str, run_query: &RunJobQuery, user_message_raw: Option<&Box>, + job_id: Uuid, ) -> error::Result<()> { + // Names the query parameter rather than the field: it is not a flow argument, and + // supplying it as one is the first thing tried on reading `memory_id is required`. let memory_id = run_query.memory_id.ok_or_else(|| { windmill_common::error::Error::BadRequest( - "memory_id is required for chat-enabled flows".to_string(), + "memory_id is required for chat-enabled flows. Pass it as the `memory_id` query \ + parameter, not as a flow argument: it names the conversation the turn belongs to, \ + so a fresh UUID starts one and reusing a UUID continues it." + .to_string(), ) })?; @@ -698,10 +704,13 @@ pub async fn handle_chat_conversation_messages( ) .await?; + // The run this message started. Its args are the only record of what the message + // carried besides its text — attachments and every other flow input — and nothing + // written later points at them: an assistant row holds the AI agent step's job. add_message_to_conversation_tx( tx, memory_id, - None, + Some(job_id), &user_message, MessageType::User, None, @@ -826,6 +835,7 @@ pub async fn run_flow<'c>( &flow_path.to_string(), &run_query, args.args.get("user_message"), + uuid, ) .await?; } diff --git a/backend/windmill-api-jobs/src/jobs_export.rs b/backend/windmill-api-jobs/src/jobs_export.rs index 05f5a51bbc..6389134031 100644 --- a/backend/windmill-api-jobs/src/jobs_export.rs +++ b/backend/windmill-api-jobs/src/jobs_export.rs @@ -692,16 +692,64 @@ pub async fn delete_jobs( .await? .rows_affected(); - let conversation_message_deleted = sqlx::query!( + // One row per message deleted, so the conversation of a chat losing several appears + // several times: the count is taken before the dedup below. + let mut conversation_ids: Vec = sqlx::query_scalar!( "DELETE FROM flow_conversation_message m USING flow_conversation c - WHERE m.conversation_id = c.id AND c.workspace_id = $1 AND m.job_id = ANY($2)", + WHERE m.conversation_id = c.id AND c.workspace_id = $1 AND m.job_id = ANY($2) + RETURNING m.conversation_id", &w_id, &job_ids ) - .execute(&mut *tx) - .await? - .rows_affected(); + .fetch_all(&mut *tx) + .await?; + let conversation_message_deleted = conversation_ids.len() as u64; + + // Same rule, lock and statement order as retention (windmill_common::jobs::delete_jobs, + // which says why): a conversation with no messages left goes, and its memory with it. + conversation_ids.sort_unstable(); + conversation_ids.dedup(); + let mut memory_deleted = 0; + let mut conversation_deleted = 0; + if !conversation_ids.is_empty() { + sqlx::query_scalar!( + "SELECT id FROM flow_conversation WHERE id = ANY($1) AND workspace_id = $2 ORDER BY id FOR UPDATE", + &conversation_ids, + &w_id + ) + .fetch_all(&mut *tx) + .await?; + memory_deleted = sqlx::query!( + "DELETE FROM ai_agent_memory a + USING flow_conversation c + WHERE c.id = ANY($1) + AND c.workspace_id = $2 + AND a.conversation_id = c.id + AND a.workspace_id = c.workspace_id + AND NOT EXISTS ( + SELECT 1 FROM flow_conversation_message m WHERE m.conversation_id = c.id + )", + &conversation_ids, + &w_id + ) + .execute(&mut *tx) + .await? + .rows_affected(); + conversation_deleted = sqlx::query!( + "DELETE FROM flow_conversation c + WHERE c.id = ANY($1) + AND c.workspace_id = $2 + AND NOT EXISTS ( + SELECT 1 FROM flow_conversation_message m WHERE m.conversation_id = c.id + )", + &conversation_ids, + &w_id + ) + .execute(&mut *tx) + .await? + .rows_affected(); + } // Resolutions are not exported, so a delete-then-reimport of the same UUID would // otherwise resurrect the old annotation on a job that never carried one. @@ -737,6 +785,8 @@ pub async fn delete_jobs( + zombie_deleted + dispatch_event_deleted + conversation_message_deleted + + memory_deleted + + conversation_deleted + resolution_deleted + jobs_deleted; diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index e388516ca7..2bfac244f5 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -3902,8 +3902,8 @@ async fn update_token_label( Path(token_prefix): Path, Json(req): Json, ) -> Result { - // The new label must not collide with a system-token namespace (`session`, - // `ephemeral*`, `debugger-token`, `mcp-oauth-*`): those labels are + // The new label must not collide with a system-token namespace (see + // `windmill_common::auth::is_user_token`): those labels are // load-bearing, and a user-set collision would orphan the token — hidden // from the UI (`isUserToken`) and rejected by the editability guard below — // while it still authenticates. (`is_user_token(None)` is true, so clearing @@ -3942,6 +3942,9 @@ async fn update_token_label( AND lower(label) NOT LIKE 'ephemeral%' AND label <> 'debugger-token' AND label NOT LIKE 'mcp-oauth-%' + AND NOT starts_with(label, 'embed_app:') + AND NOT starts_with(label, 'sdk_app:') + AND NOT starts_with(label, 'impersonation:') )) RETURNING token_prefix", req.label.as_deref(), diff --git a/backend/windmill-api-workspaces/src/datatable_acl.rs b/backend/windmill-api-workspaces/src/datatable_acl.rs index 39fa8fac5d..6bd14316fc 100644 --- a/backend/windmill-api-workspaces/src/datatable_acl.rs +++ b/backend/windmill-api-workspaces/src/datatable_acl.rs @@ -36,12 +36,10 @@ use windmill_common::datatable_roles::{ ADMIN_DATATABLE_ROLE, CUSTOM_INSTANCE_USER, }; use windmill_common::error::{pg_error_message, Error, JsonResult, Result}; -use windmill_common::workspaces::{ - get_datatable_resource_from_db_unchecked, resolve_governing_datatable, GoverningDatatable, -}; +use windmill_common::workspaces::{resolve_governing_datatable, DataTable, GoverningDatatable}; use windmill_common::{PgDatabase, DB}; -use crate::datatable_permissions::{ensure_governs_datatable, ensure_reaches_datatable}; +use crate::datatable_permissions::{ensure_governs_datatable, ensure_reaches_governing_datatable}; pub(crate) fn routes() -> Router { Router::new() @@ -322,11 +320,20 @@ async fn connect_as_admin_unchecked( mpsc::UnboundedReceiver, String, )> { - let resource = - get_datatable_resource_from_db_unchecked(db, &governing.workspace_id, &governing.name) - .await?; - let pg: PgDatabase = serde_json::from_value(resource) - .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {e}")))?; + ensure_instance(governing)?; + // Built from the authorized entry, never by resolving the settings again: a save in between + // could point the entry at a resource on another server and back, and this connection would + // then alter a database the later checks of the entry never see. + let mut pg = PgDatabase::parse_uri(&windmill_common::get_database_url().await?.as_str().await)?; + pg.dbname = governing + .datatable + .database + .as_ref() + .expect("a governing entry owns a database") + .resource_path + .clone(); + pg.user = Some(CUSTOM_INSTANCE_USER.to_string()); + pg.password = Some(windmill_common::utils::get_custom_pg_instance_password(db).await?); let dbname = pg.dbname.clone(); let (client, mut connection) = pg.connect(Some(db)).await?; // Unbounded: the driver must never wait on the receiver, which only drains once the statement @@ -1011,8 +1018,8 @@ async fn get_datatable_acl( ) -> JsonResult { crate::datatable_acl_oss::ensure_datatable_acl_available()?; let target: AclTarget = query.try_into()?; - ensure_reaches_datatable(&db, &w_id, &datatable_name, &authed).await?; let governing = resolve_governing_datatable(&db, &w_id, &datatable_name).await?; + ensure_reaches_governing_datatable(&db, &w_id, &datatable_name, &governing, &authed).await?; ensure_instance(&governing)?; let editable = ensure_governs_datatable(&db, &authed, &w_id, &governing) .await @@ -1307,6 +1314,76 @@ async fn authorize_acl_change( Ok(governing) } +static APPLY_SLOT: tokio::sync::Semaphore = tokio::sync::Semaphore::const_new(1); + +/// A role passes on only privileges it holds with grant option, and an instance database +/// provisioned before data table roles gave `custom_instance_user` none. Adds that option to its +/// database and `public` privileges, and nothing else: default privileges are left alone, since a +/// schema's change of owner is planned against them. Best-effort, as a grant it fails to enable is +/// refused when it runs. +async fn ensure_grant_options(client: &tokio_postgres::Client, db: &DB, dbname: &str) { + let held = client + .query_one( + "SELECT has_database_privilege(current_database(), 'CONNECT WITH GRANT OPTION') + AND has_database_privilege(current_database(), 'CREATE WITH GRANT OPTION') + AND (to_regnamespace('public') IS NULL + OR (has_schema_privilege('public', 'USAGE WITH GRANT OPTION') + AND has_schema_privilege('public', 'CREATE WITH GRANT OPTION')))", + &[], + ) + .await + .is_ok_and(|row| row.get::<_, bool>(0)); + if held { + return; + } + if let Err(e) = grant_options_as_server(db, dbname).await { + tracing::warn!("Could not enable grant options on '{dbname}': {e}"); + } +} + +/// Only the database's owner, the server's own Postgres user, can hand out an option it holds. +async fn grant_options_as_server(db: &DB, dbname: &str) -> Result<()> { + let server = PgDatabase::parse_uri(&windmill_common::get_database_url().await?.as_str().await)?; + let creds = PgDatabase { dbname: dbname.to_string(), ..server }; + let (client, connection) = creds.connect(Some(db)).await?; + let join_handle = tokio::spawn(async move { connection.await }); + let role = quote_ident(CUSTOM_INSTANCE_USER); + let result = client + .batch_execute(&format!( + "GRANT CONNECT, CREATE ON DATABASE {} TO {role} WITH GRANT OPTION; + DO $$ BEGIN + IF to_regnamespace('public') IS NOT NULL THEN + GRANT USAGE, CREATE ON SCHEMA public TO {role} WITH GRANT OPTION; + END IF; + END $$;", + quote_ident(dbname) + )) + .await; + drop(client); + windmill_common::shutdown_pg_connection(join_handle).await?; + result.map_err(|e| { + Error::internal_err(format!( + "Failed to grant options on '{dbname}': {}", + pg_error_message(&e) + )) + }) +} + +/// Whether the governing entry an apply was authorized on is still the one in the settings, read +/// under the lock: a save in between could have pointed it at another database or changed its roles. +fn entry_unchanged(governing: &GoverningDatatable, entry_now: Option) -> bool { + let Some(Ok(now)) = entry_now.map(serde_json::from_value::) else { + return false; + }; + match ( + serde_json::to_value(&now), + serde_json::to_value(&governing.datatable), + ) { + (Ok(now), Ok(authorized)) => now == authorized, + _ => false, + } +} + /// Plan one change against the catalog and the database as they are now. async fn build_plan( client: &tokio_postgres::Client, @@ -1502,26 +1579,37 @@ async fn apply_datatable_acl( .to_string(), ) })?; - // Refuses without taking a lock; everything is checked again once they are held. + // Everything that needs the pool happens before the locks: once `tx` holds them, a second pool + // connection could wait forever on a pool that concurrent applies, queued on the same locks, + // have exhausted. let governing = authorize_acl_change(&db, &authed, &w_id, &datatable_name).await?; + // Applies queue on an instance-wide lock while each holds a direct connection to the instance's + // Postgres; unbounded, the queue alone could exhaust its connection limit. One at a time per + // server, and the ones waiting hold no connection at all. + let _slot = APPLY_SLOT + .acquire() + .await + .map_err(|e| Error::internal_err(format!("ACL apply slot closed: {e}")))?; + let (mut client, mut notices, dbname) = connect_as_admin_unchecked(&db, &governing).await?; + ensure_grant_options(&client, &db, &dbname).await; // Held until the change is committed: a role renamed or dropped meanwhile would change what // the plan names, and a settings save could move the entry onto another database. Taken in the // same order as the permissions save, so the two cannot deadlock. let mut tx = db.begin().await?; lock_role_catalog(&mut tx).await?; - sqlx::query!( - "SELECT 1 AS one FROM workspace_settings WHERE workspace_id = $1 FOR UPDATE", - &governing.workspace_id + let entry_now = sqlx::query_scalar::<_, Option>( + "SELECT datatable->'datatables'->$2 FROM workspace_settings WHERE workspace_id = $1 FOR UPDATE", ) + .bind(&governing.workspace_id) + .bind(&governing.name) .fetch_optional(&mut *tx) - .await?; - let governing = authorize_acl_change(&db, &authed, &w_id, &datatable_name).await?; + .await? + .flatten(); let catalog = read_role_catalog_tx(&mut tx).await?; - let (mut client, mut notices, dbname) = connect_as_admin_unchecked(&db, &governing).await?; let plan = build_plan(&client, &dbname, &catalog, &req.target, &req.change).await?; - if &plan.statements != confirmed { + if !entry_unchanged(&governing, entry_now) || &plan.statements != confirmed { return Err(Error::BadRequest( "The data table or its roles changed since this was planned, so it would no longer \ run what was confirmed. Plan it again." @@ -1529,14 +1617,6 @@ async fn apply_datatable_acl( )); } - // Postgres only lets a role pass on a privilege it holds with grant option, and an instance - // database provisioned before data table roles holds none. Best-effort: a grant this fails to - // enable is refused below rather than skipped. - if let Err(e) = windmill_common::ensure_instance_db_grant_options_unchecked(&db, &dbname).await - { - tracing::warn!("Could not refresh grant options on '{dbname}': {e}"); - } - // One transaction: a half-applied ownership transfer leaves one schema's objects owned by two // different roles. let pg_tx = client.transaction().await.map_err(|e| { diff --git a/backend/windmill-api-workspaces/src/datatable_migrations.rs b/backend/windmill-api-workspaces/src/datatable_migrations.rs index 411fef9c3f..7ce6d17e07 100644 --- a/backend/windmill-api-workspaces/src/datatable_migrations.rs +++ b/backend/windmill-api-workspaces/src/datatable_migrations.rs @@ -174,6 +174,10 @@ async fn datatable_database_arg( // default role — which is what `ensure_migration_role_allowed` gated it as, and which is the // only role a DDL statement can be expected to succeed under. A migration that does declare a // role overrides this: the annotation wins over the reference. + // + // A legacy name containing `?` cannot be migrated through this reference: the appended query + // makes it neither an exact name nor a parseable one. Accepted on purpose, since such names can + // no longer be created and none are expected to carry migrations. Ok(to_raw_value(&format!( "datatable://{datatable_name}?role={ADMIN_DATATABLE_ROLE}" ))) diff --git a/backend/windmill-api-workspaces/src/datatable_permissions.rs b/backend/windmill-api-workspaces/src/datatable_permissions.rs index 5cb1f3c1c3..69e04e328d 100644 --- a/backend/windmill-api-workspaces/src/datatable_permissions.rs +++ b/backend/windmill-api-workspaces/src/datatable_permissions.rs @@ -65,3 +65,15 @@ pub(crate) async fn ensure_reaches_datatable( ) -> Result<()> { roles::ensure_reaches_datatable(db, w_id, datatable_name, authed).await } + +/// [`ensure_reaches_datatable`] against an entry already resolved, for a caller that goes on to +/// connect from that same entry. +pub(crate) async fn ensure_reaches_governing_datatable( + db: &DB, + w_id: &str, + datatable_name: &str, + governing: &GoverningDatatable, + authed: &ApiAuthed, +) -> Result<()> { + roles::ensure_reaches_governing_datatable(db, w_id, datatable_name, governing, authed).await +} diff --git a/backend/windmill-api-workspaces/src/datatable_permissions_oss.rs b/backend/windmill-api-workspaces/src/datatable_permissions_oss.rs index b94d72afa9..736493cc2d 100644 --- a/backend/windmill-api-workspaces/src/datatable_permissions_oss.rs +++ b/backend/windmill-api-workspaces/src/datatable_permissions_oss.rs @@ -12,8 +12,9 @@ #[cfg(all(feature = "private", feature = "enterprise"))] pub(crate) use crate::datatable_permissions_ee::{ - ensure_governs_datatable, ensure_reaches_datatable, get_datatable_permissions, - list_usable_datatable_roles, set_datatable_permissions, usable_datatable_roles, + ensure_governs_datatable, ensure_reaches_datatable, ensure_reaches_governing_datatable, + get_datatable_permissions, list_usable_datatable_roles, set_datatable_permissions, + usable_datatable_roles, }; #[cfg(not(all(feature = "private", feature = "enterprise")))] @@ -56,6 +57,20 @@ mod ce { } } + pub(crate) async fn ensure_reaches_governing_datatable( + _db: &DB, + _w_id: &str, + _datatable_name: &str, + governing: &GoverningDatatable, + _authed: &ApiAuthed, + ) -> Result<()> { + if governing.datatable.permissions.is_none() { + Ok(()) + } else { + Err(unavailable()) + } + } + // The routes stay registered so the API has one shape; each answers after authentication, // before anything is read. diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 0908343fff..863ea9e670 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -1316,26 +1316,22 @@ async fn get_git_sync_deploy_mode( let configured = !settings.repositories.is_empty(); - // Auto-pull runs only on Enterprise-licensed instances (see poll_git_auto_pull); - // without a caller branch there is nothing to match. Either way deploy_on_push - // stays false and the caller falls back (git push via CI, or wmill sync push). + // Auto-pull runs only in builds that compile the poller (`private`); without a + // caller branch there is nothing to match. Either way deploy_on_push stays + // false and the caller falls back (git push via CI, or wmill sync push). let Some(branch) = q.branch.as_deref() else { return Ok(Json(GitSyncDeployMode { configured, deploy_on_push: false, })); }; - let licensed = matches!( - windmill_common::ee_oss::get_license_plan().await, - windmill_common::ee_oss::LicensePlan::Enterprise - ); // Count the auto-pull repos that would deploy this branch. We deliberately do // not check the caller's remote URL: with exactly one such repo the local // checkout is unambiguously it, and with several we can't tell which is the // caller's, so we report false and let the CLI ask the user. let mut matches = 0u32; - if licensed && !root_deleted { + if cfg!(feature = "private") && !root_deleted { for repo in &settings.repositories { let Some(auto_pull) = repo.auto_pull.as_ref() else { continue; @@ -4067,50 +4063,64 @@ async fn edit_datatable_config( // entry through a declared rename alone, and a settings sync never declares one, so an entry // without roles that newly points at such a database — a name added, or an existing one // repointed — would answer everyone there as `admin`. That holds whichever workspace governs it. - let governed_elsewhere: Vec = sqlx::query_scalar( - "SELECT DISTINCT dt.value->'database'->>'resource_path' FROM workspace_settings ws - CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt - WHERE ws.workspace_id <> $1 AND dt.value ? 'permissions' - AND dt.value->'database'->>'resource_type' = 'instance'", + let newly_pointed: Vec<(&String, &str)> = new_config + .settings + .datatables + .iter() + .filter(|(_, dt)| dt.permissions.is_none()) + .filter_map(|(name, dt)| { + let db = dt + .database + .as_ref() + .filter(|d| d.resource_type == DataTableCatalogResourceType::Instance)?; + let lookup = rename_src + .get(name.as_str()) + .copied() + .unwrap_or(name.as_str()); + let repointed = old_datatables + .get(lookup) + .and_then(|old| old.database.as_ref()) + .is_none_or(|old_db| { + old_db.resource_type != db.resource_type + || old_db.resource_path != db.resource_path + }); + repointed.then_some((name, db.resource_path.as_str())) + }) + .collect(); + // Another workspace turning roles on for the same database holds only its own settings row, so + // without this the scan below could read past its uncommitted write. + windmill_common::datatable_roles::lock_instance_databases_governance( + &mut *tx, + newly_pointed.iter().map(|(_, dbname)| *dbname), ) - .bind(&w_id) - .fetch_all(&mut *tx) .await?; - for (name, dt) in new_config.settings.datatables.iter() { - if dt.permissions.is_some() { - continue; - } - let Some(db) = dt - .database - .as_ref() - .filter(|d| d.resource_type == DataTableCatalogResourceType::Instance) - else { - continue; - }; - let lookup = rename_src - .get(name.as_str()) - .copied() - .unwrap_or(name.as_str()); - let repointed = old_datatables - .get(lookup) - .and_then(|old| old.database.as_ref()) - .is_none_or(|old_db| { - old_db.resource_type != db.resource_type || old_db.resource_path != db.resource_path - }); + let governed_elsewhere: Vec = if newly_pointed.is_empty() { + vec![] + } else { + sqlx::query_scalar( + "SELECT DISTINCT dt.value->'database'->>'resource_path' FROM workspace_settings ws + CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt + WHERE ws.workspace_id <> $1 AND dt.value ? 'permissions' + AND dt.value->'database'->>'resource_type' = 'instance'", + ) + .bind(&w_id) + .fetch_all(&mut *tx) + .await? + }; + for (name, dbname) in newly_pointed { let governed_here = old_datatables.values().any(|old| { old.permissions.is_some() && old.database.as_ref().is_some_and(|d| { d.resource_type == DataTableCatalogResourceType::Instance - && d.resource_path == db.resource_path + && d.resource_path == dbname }) }); - if repointed && (governed_here || governed_elsewhere.contains(&db.resource_path)) { + if governed_here || governed_elsewhere.iter().any(|g| g == dbname) { return Err(Error::BadRequest(format!( - "Data table '{name}' would point at database '{}', which a data table under roles \ - uses, without carrying those roles: everyone reaching '{name}' would connect there \ - as `admin`. Rename the data table under roles from the data table settings, which \ - carries its roles, or turn its roles off first.", - db.resource_path + "Data table '{name}' would point at database '{dbname}', which a data table under \ + roles uses, without carrying those roles: everyone reaching '{name}' would connect \ + there as `admin`. Rename the data table under roles from the data table settings, \ + which carries its roles, or turn its roles off first." ))); } } @@ -4285,54 +4295,6 @@ fn cleanup_legacy_git_sync_settings_in_memory( #[cfg(not(feature = "enterprise"))] const CE_GIT_SYNC_MAX_USERS: i64 = 2; -/// Auto-pull is licensed per plan, not just per build: the poller only serves -/// Enterprise plans at runtime, so the save path must reject the setting too — -/// otherwise an EE binary without the plan could still register a webhook and -/// receive webhook-driven pulls. -#[cfg(feature = "enterprise")] -async fn check_git_sync_ee_license(feature: &str) -> Result<()> { - if !matches!( - windmill_common::ee_oss::get_license_plan().await, - windmill_common::ee_oss::LicensePlan::Enterprise - ) { - return Err(Error::BadRequest(format!( - "{feature} requires an Enterprise license" - ))); - } - Ok(()) -} - -#[cfg(feature = "enterprise")] -async fn check_auto_pull_license() -> Result<()> { - check_git_sync_ee_license("Automatic pull from git").await -} - -/// In-app PR creation (promotion/fork deploy branches) drives GitHub API calls -/// from the deploy completion hook; runtime-gate it like auto-pull. -#[cfg(feature = "enterprise")] -async fn check_open_prs_license<'a>( - mut repos: impl Iterator, -) -> Result<()> { - if repos.any(|r| r.promotion_open_prs || r.fork_open_prs) { - check_git_sync_ee_license("Opening pull requests from Windmill").await?; - } - Ok(()) -} - -/// Promotion mode (`use_individual_branch`: per-item `wm_deploy/**` deploy -/// branches) is an EE feature; runtime-gate it like auto-pull and PR creation -/// so an enterprise binary without an active plan can't enable it via either -/// git-sync edit endpoint. -#[cfg(feature = "enterprise")] -async fn check_promotion_license<'a>( - mut repos: impl Iterator, -) -> Result<()> { - if repos.any(|r| r.use_individual_branch.unwrap_or(false)) { - check_git_sync_ee_license("Promotion mode").await?; - } - Ok(()) -} - /// Promotion on a dev workspace needs the dev-aware sync script (hub >= 28796): /// an older pinned script bundles a CLI that force-disables per-item branches /// on every fork, so enabling promotion would silently keep deploying to the @@ -4584,18 +4546,6 @@ async fn edit_git_sync_config( )); } #[cfg(feature = "enterprise")] - if git_sync_settings - .repositories - .iter() - .any(|r| r.auto_pull.as_ref().is_some_and(|a| a.enabled)) - { - check_auto_pull_license().await?; - } - #[cfg(feature = "enterprise")] - check_open_prs_license(git_sync_settings.repositories.iter()).await?; - #[cfg(feature = "enterprise")] - check_promotion_license(git_sync_settings.repositories.iter()).await?; - #[cfg(feature = "enterprise")] check_dev_promotion_script_version(&db, &w_id, git_sync_settings.repositories.iter()) .await?; #[cfg(all(feature = "enterprise", feature = "private"))] @@ -4833,19 +4783,6 @@ async fn edit_git_sync_repository( )); } #[cfg(feature = "enterprise")] - if new_config - .repository - .auto_pull - .as_ref() - .is_some_and(|a| a.enabled) - { - check_auto_pull_license().await?; - } - #[cfg(feature = "enterprise")] - check_open_prs_license(std::iter::once(&new_config.repository)).await?; - #[cfg(feature = "enterprise")] - check_promotion_license(std::iter::once(&new_config.repository)).await?; - #[cfg(feature = "enterprise")] check_dev_promotion_script_version(&db, &w_id, std::iter::once(&new_config.repository)).await?; #[cfg(all(feature = "enterprise", feature = "private"))] check_dev_promotion_targets_parent_repo(&db, &w_id, std::iter::once(&new_config.repository)) @@ -4951,13 +4888,6 @@ async fn edit_git_sync_repository( } _ => {} } - // The request-side license gate above only saw the submitted config; the - // preservation can resurrect an enabled auto_pull (None arm), so re-check - // the effective state before it gets written and reconciled. - #[cfg(feature = "enterprise")] - if updated.auto_pull.as_ref().is_some_and(|a| a.enabled) { - check_auto_pull_license().await?; - } *existing_repo = updated; } else { // Repository doesn't exist, add it as a new repository diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 6af6bbc37c..8b85c2919b 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -57,7 +57,7 @@ use windmill_audit::audit_oss::{audit_log, AuditAuthorable}; use windmill_audit::ActionKind; use windmill_common::{ apps::{AppScriptId, ListAppQuery, APP_WORKSPACED_ROUTE}, - auth::TOKEN_PREFIX_LEN, + auth::{APP_EMBED_TOKEN_LABEL_PREFIX, RAW_APP_SDK_TOKEN_LABEL_PREFIX, TOKEN_PREFIX_LEN}, cache::{self, future::FutureCachedExt}, db::{DbWithOptAuthed, UserDB}, error::{to_anyhow, Error, JsonResult, Result}, @@ -1522,7 +1522,10 @@ async fn mint_raw_app_sdk_token( scopes.push(windmill_api_auth::scopes::GUEST_SENTINEL.to_string()); (label, exp) } - None => (format!("sdk_app:{app_path}"), requested_exp), + None => ( + format!("{RAW_APP_SDK_TOKEN_LABEL_PREFIX}{app_path}"), + requested_exp, + ), }; let token_config = NewToken::new( Some(label), @@ -1804,7 +1807,10 @@ pub async fn mint_app_embed_token( scopes.push(windmill_api_auth::scopes::GUEST_SENTINEL.to_string()); (label, exp) } - None => (format!("embed_app:{app_path}"), requested_exp), + None => ( + format!("{APP_EMBED_TOKEN_LABEL_PREFIX}{app_path}"), + requested_exp, + ), }; let token_config = NewToken::new( Some(label), diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 600aa30487..39256e7f20 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -9559,6 +9559,7 @@ async fn run_preview_flow_job( &flow_path, &run_query, user_message.as_ref(), + uuid, ) .await?; } diff --git a/backend/windmill-common/src/auth.rs b/backend/windmill-common/src/auth.rs index 73cdfba35d..3e8eb58a14 100644 --- a/backend/windmill-common/src/auth.rs +++ b/backend/windmill-common/src/auth.rs @@ -19,10 +19,11 @@ use crate::{ }; /// Whether `label` denotes a user-created token rather than a system token -/// (`session`, `guest_session`, `ephemeral*`, `debugger-token`, `mcp-oauth-*`). System-token -/// labels are load-bearing — session cleanup, super_admin propagation, expiry -/// notifications and username overrides all key off them — so they must not be -/// user-editable. `None` (no label) is treated as a user token. +/// (`session`, `guest_session`, `ephemeral*`, `debugger-token`, `mcp-oauth-*`, +/// `embed_app:*`, `sdk_app:*`, `impersonation:*`). System-token labels are load-bearing — +/// session cleanup, super_admin propagation, expiry notifications and username overrides +/// all key off them — so they must not be user-editable. `None` (no label) is treated as +/// a user token. /// /// This is the canonical copy. When updating it, also update its mirrors: /// - the `update_token_label` editability guard (SQL `WHERE`) in @@ -40,15 +41,30 @@ pub fn is_user_token(label: Option<&str>) -> bool { && !l.to_lowercase().starts_with("ephemeral") && l != "debugger-token" && !l.starts_with("mcp-oauth-") + // Short-lived tokens the server mints per app open or per service-account + // impersonation (EE `users_ee.rs`) and nobody manages, so an expiry warning + // for one is noise. + && !l.starts_with(APP_EMBED_TOKEN_LABEL_PREFIX) + && !l.starts_with(RAW_APP_SDK_TOKEN_LABEL_PREFIX) + && !l.starts_with("impersonation:") } } } +/// Label prefix, followed by the app path, of the token an app viewer's sandboxed iframe +/// runs with. Reserved in [`is_user_token`], whose SQL and frontend mirrors spell it out. +pub const APP_EMBED_TOKEN_LABEL_PREFIX: &str = "embed_app:"; + +/// Label prefix, followed by the app path, of the token a raw app's bundle uses for the +/// frontend SDK. Reserved in [`is_user_token`], whose SQL and frontend mirrors spell it out. +pub const RAW_APP_SDK_TOKEN_LABEL_PREFIX: &str = "sdk_app:"; + /// Whether `label` belongs to a namespace only the server mints, and which therefore must be /// rejected by `create_token`. Narrower than [`is_user_token`], which also drives label -/// editability and expiry notifications and can afford to reserve more: `Ephemeral lsp token` -/// and `debugger-token` are minted by the editor and the debugger through that same handler, -/// so reserving them would break those features. +/// editability and expiry notifications and can afford to reserve more: `Ephemeral lsp token`, +/// `debugger-token` and `ephemeral-test-connection: *` are minted by the editor, the debugger +/// and object-storage connection tests through that same handler, so reserving them would +/// break those features. /// /// `username_override_from_label` trusts a label to name the entity acting only if it is in /// here, so anything added must be unmintable by a member. @@ -961,6 +977,9 @@ mod tests { assert!(!is_user_token(Some("Ephemeral lsp token"))); assert!(!is_user_token(Some("debugger-token"))); assert!(!is_user_token(Some("mcp-oauth-client"))); + assert!(!is_user_token(Some("embed_app:f/team/dashboard"))); + assert!(!is_user_token(Some("sdk_app:u/admin/raw app"))); + assert!(!is_user_token(Some("impersonation:admin@windmill.dev"))); } #[test] diff --git a/backend/windmill-common/src/datatable_roles.rs b/backend/windmill-common/src/datatable_roles.rs index 3c269d1ada..4dd9b06fde 100644 --- a/backend/windmill-common/src/datatable_roles.rs +++ b/backend/windmill-common/src/datatable_roles.rs @@ -140,6 +140,25 @@ pub async fn lock_datatable_streams(conn: &mut sqlx::PgConnection, exclusive: bo Ok(()) } +/// Whether an instance database is reached only through entries under roles is decided by two +/// writes that lock different workspaces' settings rows: turning roles on for one entry, and a +/// settings save pointing an entry without roles at the database. Each holds this for every +/// database it decides on, so neither reads past the other's uncommitted write. Held for the +/// transaction; the names are locked in sorted order so two holders cannot deadlock. +pub async fn lock_instance_databases_governance<'a>( + conn: &mut sqlx::PgConnection, + dbnames: impl IntoIterator, +) -> Result<()> { + let dbnames: std::collections::BTreeSet<&str> = dbnames.into_iter().collect(); + for dbname in dbnames { + sqlx::query("SELECT pg_advisory_xact_lock(hashtext('datatable_instance_database:' || $1))") + .bind(dbname) + .execute(&mut *conn) + .await?; + } + Ok(()) +} + /// Disclosure: returns every role's stored Postgres password in plaintext. Any server path that /// has to resolve or name a role may call it — including handlers open to a workspace member, who /// need the names — but callers MUST NOT let `pwd` reach a response, a log line, an audit record diff --git a/backend/windmill-common/src/datatable_roles_oss.rs b/backend/windmill-common/src/datatable_roles_oss.rs index 569aa7ff1f..2a3f9dda48 100644 --- a/backend/windmill-common/src/datatable_roles_oss.rs +++ b/backend/windmill-common/src/datatable_roles_oss.rs @@ -164,8 +164,8 @@ mod ce { Err(unavailable()) } - /// A data table not under roles, asked for no role, is not a role decision and passes, as it - /// did before roles existed. Anything else is refused. + /// A data table not under roles, asked for no role or for `admin`, is not a role decision and + /// passes, as it did before roles existed. Anything else is refused. pub(crate) async fn ensure_can_use_datatable_role( db: &DB, w_id: &str, @@ -175,7 +175,9 @@ mod ce { _context: &str, ) -> Result<()> { let governing = resolve_governing_datatable(db, w_id, name).await?; - if governing.datatable.permissions.is_none() && role.is_none() { + if governing.datatable.permissions.is_none() + && role.is_none_or(|r| r == crate::datatable_roles::ADMIN_DATATABLE_ROLE) + { Ok(()) } else { Err(unavailable()) diff --git a/backend/windmill-common/src/flow_conversations.rs b/backend/windmill-common/src/flow_conversations.rs index 21b1f56389..b62f768bbc 100644 --- a/backend/windmill-common/src/flow_conversations.rs +++ b/backend/windmill-common/src/flow_conversations.rs @@ -36,30 +36,20 @@ pub async fn get_or_create_conversation_with_id( title: &str, conversation_id: Uuid, ) -> Result { - // Check if conversation already exists - let existing_conversation = sqlx::query_as!( - FlowConversation, - "SELECT id, workspace_id, flow_path, title, created_at, updated_at, created_by - FROM flow_conversation - WHERE id = $1 AND workspace_id = $2", - conversation_id, - w_id - ) - .fetch_optional(&mut **tx) - .await?; - - if let Some(existing) = existing_conversation { + if let Some(existing) = lock_conversation(tx, w_id, conversation_id).await? { return Ok(existing); } // Truncate title to 25 characters max let title = truncate_with_ellipsis(title, 25); - // Create new conversation with provided ID - let conversation = sqlx::query_as!( + // Every turn released by the same collector's commit finds no row: the first insert + // wins, the others wait on it, do nothing, and read the row it created. + let created = sqlx::query_as!( FlowConversation, "INSERT INTO flow_conversation (id, workspace_id, flow_path, created_by, title) VALUES ($1, $2, $3, $4, $5) + ON CONFLICT (id) DO NOTHING RETURNING id, workspace_id, flow_path, title, created_at, updated_at, created_by", conversation_id, w_id, @@ -67,10 +57,41 @@ pub async fn get_or_create_conversation_with_id( username, title ) - .fetch_one(&mut **tx) + .fetch_optional(&mut **tx) .await?; + if let Some(conversation) = created { + return Ok(conversation); + } - Ok(conversation) + lock_conversation(tx, w_id, conversation_id) + .await? + .ok_or_else(|| { + crate::error::Error::BadRequest(format!( + "conversation {conversation_id} belongs to another workspace" + )) + }) +} + +/// Locked, so a turn orders against retention collecting the conversation +/// (windmill_common::jobs::delete_jobs): either the turn goes first and the collector then +/// sees its message, or it waits and finds the row gone and creates it again. Unlocked, the +/// message insert would wait on the parent row's lock instead and then fail its FK check. +async fn lock_conversation( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + w_id: &str, + conversation_id: Uuid, +) -> Result> { + Ok(sqlx::query_as!( + FlowConversation, + "SELECT id, workspace_id, flow_path, title, created_at, updated_at, created_by + FROM flow_conversation + WHERE id = $1 AND workspace_id = $2 + FOR UPDATE", + conversation_id, + w_id + ) + .fetch_optional(&mut **tx) + .await?) } /// Add a message to a conversation using an existing transaction diff --git a/backend/windmill-common/src/jobs.rs b/backend/windmill-common/src/jobs.rs index 5541615838..6f3248f3fb 100644 --- a/backend/windmill-common/src/jobs.rs +++ b/backend/windmill-common/src/jobs.rs @@ -478,6 +478,12 @@ pub static WORKER_INTERNAL_SERVER_INLINE_UTILS: OnceCell error::Result<()> { sqlx::query!( "DELETE FROM dispatch_event WHERE producer_job_id = ANY($1)", @@ -485,12 +491,55 @@ pub async fn delete_jobs(conn: &mut sqlx::PgConnection, ids: &[uuid::Uuid]) -> e ) .execute(&mut *conn) .await?; - sqlx::query!( - "DELETE FROM flow_conversation_message WHERE job_id = ANY($1)", + let mut conversation_ids: Vec = sqlx::query_scalar!( + "DELETE FROM flow_conversation_message WHERE job_id = ANY($1) RETURNING conversation_id", ids ) - .execute(&mut *conn) + .fetch_all(&mut *conn) .await?; + conversation_ids.sort_unstable(); + conversation_ids.dedup(); + if !conversation_ids.is_empty() { + // A conversation is a view over its messages: once the last one goes with its job, + // the row and the agent's memory for it are all that is left, and nothing else + // collects them — `ai_agent_memory` carries no job id for retention to match on. + // Two statements rather than one CTE: a data-modifying CTE reads the snapshot from + // before the delete above, so every conversation would still look non-empty. + // Two calls each deleting one of a conversation's last messages would each still see + // the other's row — uncommitted deletes are invisible across transactions — so + // neither would collect it and nothing would try again. Taking the conversation row + // first serialises them: the second reads the first's delete and finds it empty. + sqlx::query_scalar!( + "SELECT id FROM flow_conversation WHERE id = ANY($1) ORDER BY id FOR UPDATE", + &conversation_ids + ) + .fetch_all(&mut *conn) + .await?; + // Memory first, since it reads the conversation row for its workspace. + sqlx::query!( + "DELETE FROM ai_agent_memory a + USING flow_conversation c + WHERE c.id = ANY($1) + AND a.conversation_id = c.id + AND a.workspace_id = c.workspace_id + AND NOT EXISTS ( + SELECT 1 FROM flow_conversation_message m WHERE m.conversation_id = c.id + )", + &conversation_ids + ) + .execute(&mut *conn) + .await?; + sqlx::query!( + "DELETE FROM flow_conversation c + WHERE c.id = ANY($1) + AND NOT EXISTS ( + SELECT 1 FROM flow_conversation_message m WHERE m.conversation_id = c.id + )", + &conversation_ids + ) + .execute(&mut *conn) + .await?; + } sqlx::query!("DELETE FROM zombie_job_counter WHERE job_id = ANY($1)", ids) .execute(&mut *conn) .await?; diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 6183e8375a..fb6897f81a 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1686,9 +1686,10 @@ pub async fn get_datatable_resource_from_db( ) -> Result { let governing = resolve_governing_datatable(db, w_id, name).await?; let db_resource = resolve_datatable_connection_unchecked(db, &governing, false).await?; - // Not under roles and asked for none: the `admin` connection, as before roles existed, in - // every edition. Anything else is a role decision. - if governing.datatable.permissions.is_none() && role.is_none() { + // Not under roles and asked for none, or for `admin` by name: the `admin` connection, as before + // roles existed, in every edition. Anything else is a role decision. Every migration names + // `admin` explicitly, so an edition without roles must not treat that as one. + if governing.datatable.permissions.is_none() && role.is_none_or(|r| r == ADMIN_DATATABLE_ROLE) { return Ok(db_resource); } crate::datatable_roles_oss::resolve_datatable_role_connection( @@ -1898,22 +1899,34 @@ pub fn strip_datatable_permissions( /// As [`parse_datatable_ref`], except that an entry whose stored name itself contains `?` — which /// names could before they were restricted — resolves by that exact name, without a role. It is /// looked up first, so `sales?role=x` never reaches a different entry than the one stored so. +/// When `sales` is stored too, the reference means either one, and is refused rather than +/// resolved to whichever is looked up first. pub async fn parse_datatable_ref_for( db: &DB, w_id: &str, reference: &str, ) -> Result<(String, Option)> { if reference.contains('?') { - let exists = sqlx::query_scalar::<_, Option>( - "SELECT (datatable->'datatables') ? $2 FROM workspace_settings WHERE workspace_id = $1", + let role_target = parse_datatable_ref(reference) + .ok() + .and_then(|(name, role)| role.map(|_| name)); + let (exists, target_exists) = sqlx::query_as::<_, (Option, Option)>( + "SELECT (datatable->'datatables') ? $2, (datatable->'datatables') ? $3 + FROM workspace_settings WHERE workspace_id = $1", ) .bind(w_id) .bind(reference) + .bind(role_target) .fetch_optional(db) .await? - .flatten() - .unwrap_or(false); - if exists { + .unwrap_or((None, None)); + if exists.unwrap_or(false) { + if let (Some(name), Some(true)) = (role_target, target_exists) { + return Err(Error::BadRequest(format!( + "Data table reference '{reference}' names both the data table '{reference}' \ + and a role on the data table '{name}'. Rename '{reference}' to use either." + ))); + } return Ok((reference.to_string(), None)); } } diff --git a/backend/windmill-worker/src/result_processor.rs b/backend/windmill-worker/src/result_processor.rs index 1529d474fa..0490ed1754 100644 --- a/backend/windmill-worker/src/result_processor.rs +++ b/backend/windmill-worker/src/result_processor.rs @@ -1254,18 +1254,6 @@ async fn maybe_open_git_sync_deploy_pr( if row.marker.is_none() { return; } - // Runtime Enterprise gate, like the poller: the toggles may have been set - // while a license was active (or written directly), and this hook drives - // GitHub API calls with the installation token. - if !matches!( - windmill_common::ee_oss::get_license_plan().await, - windmill_common::ee_oss::LicensePlan::Enterprise - ) { - tracing::warn!( - "git sync PR: skipping PR creation for {workspace_id}: requires an Enterprise license" - ); - return; - } let Some(repo_path) = row.repo_path else { return; }; diff --git a/cli/src/guidance/skills.gen.ts b/cli/src/guidance/skills.gen.ts index 004c3d728f..551757bb30 100644 --- a/cli/src/guidance/skills.gen.ts +++ b/cli/src/guidance/skills.gen.ts @@ -5294,9 +5294,60 @@ tool, \`websearch\` for web search. } \`\`\` -- \`provider\` is a static object, not a bare resource string: \`{ "kind": , +- \`provider\` is an object, not a bare resource string: \`{ "kind": , "resource": "$res:", "model": }\`. Required unless the module links to a saved - agent through \`value.agent\` + agent through \`value.agent\`. Static is right for a flow run from a form; a chat flow wires its + fields to flow inputs instead — see below + +### Chat-Mode Flows + +A flow with \`value.chat_input_enabled: true\` is run from a chat instead of a form: the composer +sends one message per turn and renders the conversation. It needs a required \`user_message\` string +input, read by the agent. Any other flow input the composer does not edit itself is asked for +under Configure inputs. + +**A static \`provider\` gives a chat that cannot change its model.** Feed it from flow inputs +instead, either way round: one input carrying the whole object (\`"expr": "flow_input.model_config"\`) +makes every field editable, or wire it field by field to fix some and expose others. A field the +chat can write becomes a control in the composer — a provider picker, a model list, a thinking +control — and a field left static is fixed, with no control drawn for it. \`kind\` is the one +exception: the composer writes it only together with \`resource\`, since a provider is picked as a +pair, so a \`kind\` input wired on its own stays askable under Configure inputs and nothing the run +needs becomes unreachable. + +\`\`\`json +{ + "id": "chat_agent", + "value": { + "type": "aiagent", + "input_transforms": { + "provider": { + "type": "javascript", + "expr": "({ kind: 'anthropic', resource: '$res:f/ai/claude', model: flow_input.model, reasoning_effort: flow_input.thinking })" + }, + "user_message": { "type": "javascript", "expr": "flow_input.user_message" }, + "user_attachments": { "type": "javascript", "expr": "flow_input.files" }, + "memory": { "type": "static", "value": { "kind": "auto", "context_length": 10 } }, + "streaming": { "type": "static", "value": true }, + "output_type": { "type": "static", "value": "text" } + }, + "tools": [] + } +} +\`\`\` + +- Wiring field by field means one object literal whose values are literals or bare \`flow_input.x\` + references. A spread, a call or a computed key leaves the composer unable to tell which input + feeds which field, so it offers no control at all — a bare \`flow_input.x\` for the whole object + is read instead as that one input carrying every field +- \`memory\` is what lets the agent see earlier turns; without it every message starts from nothing +- \`streaming\` on makes the answer and its thinking appear token by token instead of all at once +- \`user_attachments\` points at a flow input typed as an array of s3 objects + (\`{ "type": "array", "items": { "type": "object", "resourceType": "s3object" } }\`), so files + sent with a message reach the agent +- Running one needs a \`memory_id\` **query parameter** — not a flow argument — naming the + conversation the turn belongs to: a fresh UUID starts one, reusing a UUID continues it. The chat + supplies it itself; a run driven any other way has to pass it or the server refuses the job ### Tool Naming Rules diff --git a/docs/auth-surface.md b/docs/auth-surface.md index 0876a1184a..abeba1fe2b 100644 --- a/docs/auth-surface.md +++ b/docs/auth-surface.md @@ -12,6 +12,14 @@ Symbols, not line numbers, are cited: they drift less. by `create_session_token` (`windmill-api-users/src/users.rs`). `GET /api/users/refresh_token` mints one for any non-job token but returns plain text, no redirect. - **`tokens/impersonate`** (superadmin) returns a multi-use token and sets no cookie. +- **A token's label decides whether its expiry raises alerts.** When `delete_expired_items` + removes an expired `token` row, the monitor emails the owner and raises a critical alert (if + enabled); rows registered by `register_token_expiry_notification` also get an "expiring soon" + warning first. Neither happens when `is_user_token` (`windmill-common/src/auth.rs`) reserves + the label, so a token the system mints for itself, whether from the backend or from the frontend + through `tokens/create`, needs a reserved label. An `ephemeral-` prefix needs no other change + (keep it clear of `is_server_minted_label` if minted through `tokens/create`); a new prefix + also goes into the SQL and Svelte mirrors that function's doc lists. - **Every superadmin route refuses a job token**: `require_super_admin` (`windmill-api-auth/src/lib.rs`) errors on `authed.job_id.is_some()`. A script that needs `users/create`, `tokens/impersonate`, `set_login_type`, … must use a dedicated superadmin user diff --git a/frontend/src/lib/common.ts b/frontend/src/lib/common.ts index 0d76b1a0b8..4f82fd252e 100644 --- a/frontend/src/lib/common.ts +++ b/frontend/src/lib/common.ts @@ -45,6 +45,8 @@ export interface SchemaProperty { required?: string[] showExpr?: string hideWhenChatEnabled?: boolean + /** Why the oneOf variant is chat mode's to pick. Set = selector disabled, reason shown. */ + lockOneOfWhenChatEnabled?: string password?: boolean order?: string[] nullable?: boolean diff --git a/frontend/src/lib/components/AIReasoningEffortPicker.svelte b/frontend/src/lib/components/AIReasoningEffortPicker.svelte index 3ebf94c9bf..39b4e80011 100644 --- a/frontend/src/lib/components/AIReasoningEffortPicker.svelte +++ b/frontend/src/lib/components/AIReasoningEffortPicker.svelte @@ -27,7 +27,7 @@ let capability = $derived( provider && model ? getReasoningCapability(provider, model) - : { supported: false, levels: [], canDisable: false } + : { supported: false, levels: [], canDisable: false, known: false } ) // The token that turns reasoning off on a model that reasons by default diff --git a/frontend/src/lib/components/ArgInput.svelte b/frontend/src/lib/components/ArgInput.svelte index b7001059c2..eaca31c761 100644 --- a/frontend/src/lib/components/ArgInput.svelte +++ b/frontend/src/lib/components/ArgInput.svelte @@ -123,6 +123,8 @@ workspace?: string | undefined s3StorageConfigured?: boolean chatInputEnabled?: boolean + /** Why the oneOf variant is fixed. Set = the selector is disabled and says so. */ + oneOfLockedReason?: string actions?: import('svelte').Snippet innerBottomSnippet?: import('svelte').Snippet fieldHeaderActions?: import('svelte').Snippet @@ -184,6 +186,7 @@ workspace = undefined, s3StorageConfigured = true, chatInputEnabled = false, + oneOfLockedReason = undefined, actions, innerBottomSnippet, fieldHeaderActions, @@ -1104,11 +1107,15 @@ {:else if inputCat == 'object' || inputCat == 'resource-object' || isListJson} {#if oneOf && oneOf.length >= 2}
+ {#if oneOfLockedReason !== undefined} +
{oneOfLockedReason}
+ {/if} {#if oneOf && oneOf.length >= 2} { oneOfSelected = detail const selectedObjProperties = diff --git a/frontend/src/lib/components/DBManagerDrawer.svelte b/frontend/src/lib/components/DBManagerDrawer.svelte index 330449eb7e..71dda5e1d3 100644 --- a/frontend/src/lib/components/DBManagerDrawer.svelte +++ b/frontend/src/lib/components/DBManagerDrawer.svelte @@ -17,6 +17,7 @@ import { ADMIN_DATATABLE_ROLE, datatableNameTakesRole, + defaultMigrationRole, type DatatableRowAction } from './dbTypes' import ResourcePicker from './ResourcePicker.svelte' @@ -103,6 +104,17 @@ if (effective) untrack(() => (uriState.selectedRole = effective)) }) + const contentInput = $derived.by(() => { + const input = uriState.effectiveInput + if (input?.type !== 'database' || selectedDatatable === undefined) return input + const migrationRole = defaultMigrationRole( + selectedDatatable, + rolesOfCurrent?.permissioned, + rolesOfCurrent?.default_role + ) + return migrationRole === undefined ? input : { ...input, migrationRole } + }) + // Every data table with its schemas and tables, in one call: this is what the // left pane's tree navigates, so it has to cover the data tables the user is // not currently on, not just the selected one. The privileges it reports are @@ -295,11 +307,11 @@ noPadding id="db-manager-drawer" > - {#if uriState.effectiveInput && ws && roleSettled} + {#if contentInput && ws && roleSettled} {#key `${selectedDatatable}~${selectedRole ?? ''}`} shouldHideNoInputs?: boolean noVariablePicker?: boolean @@ -89,6 +91,7 @@ let { schema = $bindable(), hiddenArgs = [], + lockedArgs = [], args = $bindable(undefined), shouldHideNoInputs = false, noVariablePicker = false, @@ -587,6 +590,7 @@ > {#if keys.length > 0} {#each keys as argName, i (argName)} + {@const locked = lockedArgs.includes(argName)}
@@ -605,7 +609,7 @@ >
{argName} - {#if !uiOnly} + {#if !uiOnly && !locked}
{#snippet trigger()} @@ -654,7 +658,7 @@ Required {/if} - {#if !uiOnly} + {#if !uiOnly && !locked}
{:else if stepDetail == 'Input'} + {#if schema} {:else}

No input schema

{/if} {:else if stepDetail == 'Result'} +

End of the flow

{:else if typeof stepDetail != 'string' && stepDetail.value} + +
-
- {#if stepDetail.id && stepDetail.id != 'failure' && stepDetail.id != 'preprocessor'} - - {stepDetail.id} - - {/if} - - {#if stepDetail.summary} - {stepDetail.summary} - {:else if stepDetail.value.type == 'identity'} - Identity - {:else if stepDetail.value.type == 'forloopflow'} - For loop {#if stepDetail.value.parallel}(parallel){/if} - {#if stepDetail.value.skip_failures}(skip failures){/if} - {#if stepDetail.value.squash}(squash){/if} - {:else if stepDetail.value.type == 'branchall'} - Run all branches {#if stepDetail.value.parallel}(parallel){/if} - {:else if stepDetail.value.type == 'branchone'} - Run one branch - {:else if stepDetail.value.type == 'flow'} - Inner flow - {:else if stepDetail.value.type == 'whileloopflow'} - While loop {#if stepDetail.value.skip_failures}(skip failures){/if} - {#if stepDetail.value.squash}(squash){/if} - {:else if stepDetail.id === 'failure'} - Error handler - {:else if stepDetail.id === 'preprocessor'} - Preprocessor - {:else if stepDetail.value.type == 'rawscript'} - Inline {stepDetail.value.language} script - {:else if stepDetail.value.type == 'script'} - Workspace script - {:else if stepDetail.value.type == 'aiagent'} - AI Agent - {/if} - -
{#if stepDetail.value.type == 'script'}
+ import type { FlowModule } from '$lib/gen' + import { Badge, Button } from './common' + import { ArrowLeft } from 'lucide-svelte' + + interface Props { + /** A module, or the graph's pseudo-nodes by id (`Input`, `Result`). */ + stepDetail: FlowModule | string + /** Given, the row starts with a back control; the caller decides where back leads. */ + onBack?: () => void + } + + let { stepDetail, onBack = undefined }: Props = $props() + + const module = $derived(typeof stepDetail === 'string' ? undefined : stepDetail) + // The error handler and the preprocessor are named by their role, not by an id badge. + const showId = $derived( + module?.id !== undefined && module.id !== 'failure' && module.id !== 'preprocessor' + ) + + const title = $derived.by((): string => { + if (typeof stepDetail === 'string') { + if (stepDetail === 'Input') return 'Flow inputs' + if (stepDetail === 'Result') return 'Result' + return stepDetail + } + if (stepDetail.summary) return stepDetail.summary + if (stepDetail.id === 'failure') return 'Error handler' + if (stepDetail.id === 'preprocessor') return 'Preprocessor' + const v = stepDetail.value + switch (v?.type) { + case 'identity': + return 'Identity' + case 'forloopflow': + return ( + 'For loop' + + (v.parallel ? ' (parallel)' : '') + + (v.skip_failures ? ' (skip failures)' : '') + + (v.squash ? ' (squash)' : '') + ) + case 'whileloopflow': + return ( + 'While loop' + (v.skip_failures ? ' (skip failures)' : '') + (v.squash ? ' (squash)' : '') + ) + case 'branchall': + return 'Run all branches' + (v.parallel ? ' (parallel)' : '') + case 'branchone': + return 'Run one branch' + case 'flow': + return 'Inner flow' + case 'rawscript': + return `Inline ${v.language} script` + case 'script': + return 'Workspace script' + case 'aiagent': + return 'AI Agent' + default: + return stepDetail.id + } + }) + + + +
+ {#if onBack} +
diff --git a/frontend/src/lib/components/FlowPreviewContent.svelte b/frontend/src/lib/components/FlowPreviewContent.svelte index f6f6da0cc9..cd2460b1b0 100644 --- a/frontend/src/lib/components/FlowPreviewContent.svelte +++ b/frontend/src/lib/components/FlowPreviewContent.svelte @@ -473,6 +473,7 @@ hideSidebar={true} path={$pathStore} inputSchema={flowStore.val.schema} + flowModules={flowStore.val.value?.modules} />
{:else} diff --git a/frontend/src/lib/components/InputTransformForm.svelte b/frontend/src/lib/components/InputTransformForm.svelte index c5ae65eda2..df477e2044 100644 --- a/frontend/src/lib/components/InputTransformForm.svelte +++ b/frontend/src/lib/components/InputTransformForm.svelte @@ -999,6 +999,11 @@ {helperScript} {s3StorageConfigured} {chatInputEnabled} + oneOfLockedReason={chatInputEnabled && + arg?.type === 'static' && + (arg.value as any)?.kind !== 'off' + ? schema.properties[argName]?.lockOneOfWhenChatEnabled + : undefined} otherArgs={Object.fromEntries( Object.entries(otherArgs).map(([key, transform]) => [ key, diff --git a/frontend/src/lib/components/SqlRepl.svelte b/frontend/src/lib/components/SqlRepl.svelte index 9701dd5f23..48a76f74c1 100644 --- a/frontend/src/lib/components/SqlRepl.svelte +++ b/frontend/src/lib/components/SqlRepl.svelte @@ -227,6 +227,6 @@ bind:this={ddlGuard} workspace={ws} datatable={datatableName} - role={input.type === 'database' ? input.role : undefined} + role={input.type === 'database' ? (input.role ?? input.migrationRole) : undefined} /> {/if} diff --git a/frontend/src/lib/components/TestConnection.svelte b/frontend/src/lib/components/TestConnection.svelte index 6a58af8031..80ec59d206 100644 --- a/frontend/src/lib/components/TestConnection.svelte +++ b/frontend/src/lib/components/TestConnection.svelte @@ -174,7 +174,7 @@ export async function main(bucket: any, api_token: string) { async function mintApiToken(): Promise { return await UserService.createToken({ requestBody: { - label: `test connection: ${resourceType}`, + label: `ephemeral-test-connection: ${resourceType}`, expiration: new Date(Date.now() + API_TOKEN_TTL_MS).toISOString(), scopes: ['settings:write'] } diff --git a/frontend/src/lib/components/copilot/ChatModelSettings.svelte b/frontend/src/lib/components/copilot/ChatModelSettings.svelte new file mode 100644 index 0000000000..a1e677d46b --- /dev/null +++ b/frontend/src/lib/components/copilot/ChatModelSettings.svelte @@ -0,0 +1,314 @@ + + +{#snippet trigger()} +
+ +
+{/snippet} + +{#snippet typedField( + value: string, + placeholder: string, + onCommit: (value: string) => void, + close: () => void +)} + {#key value} + onCommit(e.currentTarget.value.trim()), + // Capture, not bubble: Svelte delegates `keydown` to the root, which sits above the + // menu — so a bubble handler here would run only after melt's own listener had read + // the key as typeahead and moved focus. A capture key is not delegatable, so this + // becomes a real listener on the input and sees the event first. + onkeydowncapture: (e) => { + // Escape cancels: let it reach the menu with the value untouched. + if (e.key === 'Escape') return + // Tab closes the menu, unmounting this field before focus moves, so no change + // event would ever fire. Commit on the way past. + if (e.key === 'Tab') { + onCommit(e.currentTarget.value.trim()) + return + } + // Enter means done: commit and close, rather than leaving the menu open around a + // field the commit is about to rebuild. + if (e.key === 'Enter') { + e.preventDefault() + onCommit(e.currentTarget.value.trim()) + close() + return + } + // Everything else is typing; the menu reads loose keys as typeahead. + e.stopPropagation() + } + }} + /> + {/key} +{/snippet} + +{#snippet section(sec: ChoiceSection, item: MeltItem, close: () => void)} +
{sec.label}
+ {#if sec.loading} +
+ Loading... +
+ {:else if sec.options.length === 0} +
{sec.emptyMessage ?? 'Nothing to choose from'}
+ {:else} +
+ {#each sec.options as option (option.key)} + option.onSelect()}> + {option.label} + {#if option.hint} + {option.hint} + {/if} + {#if option.selected} + + {/if} + + {/each} +
+ {/if} + {#if sec.custom && !sec.loading} + {@const custom = sec.custom} +
+ {@render typedField( + '', + custom.placeholder, + (value) => { + if (value) custom.onCommit(value) + }, + close + )} +
+ {/if} +{/snippet} + +{#snippet rows(items: Item[], item: MeltItem, builders: MeltBuilders)} + {#each items.filter((row) => !row.hide) as row (row.displayName)} + {#if row.separatorTop} +
+ {/if} + {#if row.submenuItems} + + + {:else} + row.action?.(e)}> + {#if row.icon} + + {/if} + {row.displayName} + {#if row.selected} + + {/if} + + {/if} + {/each} +{/snippet} + +{#if config.readOnly} + {@render trigger()} +{:else} + + {#snippet buttonReplacement()} + {@render trigger()} + {/snippet} + {#snippet menu({ item, builders, close })} +
+ {#if config.topItems} +
+ {@render rows(config.topItems(close), item, builders)} +
+ {/if} + {#each config.sections ?? [] as sec (sec.label)} +
+ {@render section(sec, item, close)} +
+ {/each} + {#if reasoning} +
+ {#if controlState === 'fixed'} + {}} + unsupportedReason={fixedReason} + /> + {:else if controlState === 'awaiting-model'} + {}} + unsupportedReason="Pick a model first" + /> + {:else if controlState === 'unknown'} + +
+
Thinking
+ {@render typedField(reasoning.value ?? '', 'none', reasoning.onSelect, close)} +
+ Windmill has no thinking levels for this provider — type what it accepts. +
+
+ {:else if controlState === 'ladder'} + + effortSlider?.adjust(e)} + class="block group" + > + (stop === reasoning?.offToken ? 'off' : stop)} + overrideLabel={stops.includes(currentStop) ? undefined : effortLabel} + /> + + {:else} + + {}} + unsupportedReason="Not supported by this model" + /> + {/if} +
+ {/if} + {#if config.bottomItems} +
+ {@render rows(config.bottomItems(close), item, builders)} +
+ {/if} +
+ {/snippet} + +{/if} diff --git a/frontend/src/lib/components/copilot/ReasoningEffortSlider.svelte b/frontend/src/lib/components/copilot/ReasoningEffortSlider.svelte new file mode 100644 index 0000000000..24ea4cf8aa --- /dev/null +++ b/frontend/src/lib/components/copilot/ReasoningEffortSlider.svelte @@ -0,0 +1,172 @@ + + +{#if unsupportedReason} + +
+
Thinking
+
{unsupportedReason}
+
+{:else} +
+ Thinking + {overrideLabel ?? format(current)} +
+ {#if stops.length > 1} + +
+ onSelect(stops[+e.currentTarget.value])} + onclick={(e) => { + // `click`, not `pointerup`: it is the event that means pressed and released on + // the track, so a press that began on the row above cannot commit an effort + // nobody chose. Only the click that moved nothing — any other stop has already + // committed through `oninput`, and doing it again would write it twice. + if (!hasPosition && +e.currentTarget.value === stopIndex) { + onSelect(stops[stopIndex]) + } + }} + use:isolatePointer + class="lean-range no-default-style w-full" + aria-label="Reasoning effort" + /> +
+ {/if} +{/if} + + diff --git a/frontend/src/lib/components/copilot/ScriptFix.svelte b/frontend/src/lib/components/copilot/ScriptFix.svelte index f586ad1aaf..78c4804cc8 100644 --- a/frontend/src/lib/components/copilot/ScriptFix.svelte +++ b/frontend/src/lib/components/copilot/ScriptFix.svelte @@ -25,7 +25,7 @@ /** The failing run's error, used when there is no job to point at. */ error?: string /** The failing run's job id. Preferred over `error`: the chat reads the - * run itself with `get_job_logs`, which gives it the logs rather than + * run itself with `get_run`, which gives it the logs rather than * just the thrown value, and keeps the composer readable. */ jobId?: string /** Set when this sits in a flow step's preview, so the session opens on diff --git a/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte b/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte index 7de7dd32dc..82dcbc2dcd 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte @@ -44,6 +44,7 @@ import Markdown from 'svelte-exmarkdown' import { twMerge } from 'tailwind-merge' import { AIAutonomyMode, AIMode } from './AIChatManager.svelte' + import { getChatViewHost } from './chatViewHost' import { getAiChatManager } from './aiChatManagerContext' import ChatTypingIndicator from './ChatTypingIndicator.svelte' import AIChatInput from './AIChatInput.svelte' @@ -68,14 +69,19 @@ import { base } from '$lib/base' const MAX_YOLO_TOOLTIP_TOOLS = 8 + const chatHost = getChatViewHost() + // The skill and MCP menus take an AIChatManager itself, which the seam deliberately + // doesn't carry. They render only under GLOBAL, which a non-copilot host never sets. const aiChatManager = getAiChatManager() + // The free grant pays for the copilot's own model, so its banners belong only to a host + // that sends to that model. A flow chat's turn runs on the flow's provider. + const freeTier = $derived(chatHost.supportsModelSettings ? $copilotInfo.freeTier : undefined) // The user spent their one-time free Windmill AI grant: there is no model left to send // to, so say so in the thread itself rather than only failing on send. - let freeTierExhausted = $derived($copilotInfo.freeTier?.exhausted === true) + let freeTierExhausted = $derived(freeTier?.exhausted === true) // Still on the free grant: keep how much is left in view right above the composer, so // running out isn't a surprise. Once spent, the exhausted banner replaces it. - let freeTier = $derived($copilotInfo.freeTier) let freeTierUsedPct = $derived(Math.min(100, Math.round((freeTier?.used_ratio ?? 0) * 100))) let showFreeTierUsage = $derived(!!freeTier && !freeTier.exhausted) @@ -174,8 +180,12 @@ wideLayout = false, emptyHint, inputPreface, + footerSettings, initialInstructions = undefined, - onDraftChange = undefined + onDraftChange = undefined, + placeholder = undefined, + scrollElement = $bindable(), + onTranscriptScroll = undefined }: { messages: DisplayMessage[] pastChats: { id: string; title: string }[] @@ -202,9 +212,18 @@ wideLayout?: boolean emptyHint?: Snippet inputPreface?: Snippet + /** The settings control at the footer's right edge, where the copilot puts its + * model picker. A host that configures its turn elsewhere replaces it here. */ + footerSettings?: Snippet // Seed / observe the main composer's draft text (see AIChatInput). initialInstructions?: string onDraftChange?: (text: string) => void + /** Composer placeholder. Falls back to the per-AI-mode wording. */ + placeholder?: string + /** The transcript's scroll container. A host that paginates older messages + * needs it to measure and restore the scroll position. */ + scrollElement?: HTMLDivElement | undefined + onTranscriptScroll?: () => void } = $props() let aiChatInput: AIChatInput | undefined = $state() @@ -223,7 +242,7 @@ let panelEl: HTMLDivElement | undefined = $state() $effect(() => { function onWindowKeydownCapture(e: KeyboardEvent) { - if (e.key !== 'Escape' || !aiChatManager.loading) return + if (e.key !== 'Escape' || !chatHost.loading) return const active = document.activeElement const focusOnChat = !active || active === document.body || (panelEl?.contains(active) ?? false) @@ -231,22 +250,21 @@ // row alone stops the turn — wherever it is mounted, since the preview panel holds the // form outside `panelEl`. Matched by call: two chats can be loading at once, and one's // row must not answer for the other. - if (aiChatManager.hasPendingRunForm) { + if (chatHost.hasPendingRunForm) { const row = active?.closest('[data-run-form-actions]') const toolCallId = row?.getAttribute('data-run-form-actions') - if (!toolCallId || !aiChatManager.isRunFormPending(toolCallId)) return + if (!toolCallId || !chatHost.isRunFormPending(toolCallId)) return } else if (!focusOnChat) return e.preventDefault() // Immediate form: other chat panels' identical listeners must not // also cancel on body focus, nor a drawer/modal close on this press. e.stopImmediatePropagation() - aiChatManager.cancel() + chatHost.cancel() } window.addEventListener('keydown', onWindowKeydownCapture, true) return () => window.removeEventListener('keydown', onWindowKeydownCapture, true) }) - let scrollEl: HTMLDivElement | undefined = $state() // Programmatic-scroll guard. `scrollDown()` triggers an async `scroll` // event; if a token-append between the scrollTo and the dispatch makes // scrollHeight grow, the gap can briefly exceed STICK_TO_BOTTOM_PX and @@ -259,22 +277,23 @@ // Instant scroll — smooth would animate every token append, racing with // the next scrollDown and confusing the onscroll bottom-detection below. function scrollDown() { - if (!scrollEl) return + if (!scrollElement) return programmaticScrollAt = Date.now() - scrollEl.scrollTo({ top: scrollEl.scrollHeight, behavior: 'auto' }) + scrollElement.scrollTo({ top: scrollElement.scrollHeight, behavior: 'auto' }) } let height = $state(0) $effect(() => { - if (aiChatManager.automaticScroll && height) { + if (chatHost.automaticScroll && height) { scrollDown() } // Recompute the scroll-to-latest visibility on every content-height // change. `onScroll` only fires for actual scroll events, so without // this the arrow can go stale when content grows past the threshold // while auto-scroll is disabled (user scrolled up mid-stream). - if (scrollEl && height) { - const distance = scrollEl.scrollHeight - scrollEl.scrollTop - scrollEl.clientHeight + if (scrollElement && height) { + const distance = + scrollElement.scrollHeight - scrollElement.scrollTop - scrollElement.clientHeight showScrollToLatest = distance > SCROLL_TO_LATEST_THRESHOLD_PX } }) @@ -289,8 +308,9 @@ const SCROLL_TO_LATEST_THRESHOLD_PX = 200 let showScrollToLatest = $state(false) function onScroll() { - if (!scrollEl) return - const distance = scrollEl.scrollHeight - scrollEl.scrollTop - scrollEl.clientHeight + if (!scrollElement) return + const distance = + scrollElement.scrollHeight - scrollElement.scrollTop - scrollElement.clientHeight // Always refresh the arrow visibility — even during the cooldown, // because clicking the arrow itself triggers a programmatic scroll // whose only event would otherwise be swallowed, leaving the arrow @@ -303,14 +323,15 @@ return } if (distance <= STICK_TO_BOTTOM_PX) { - aiChatManager.enableAutomaticScroll() + chatHost.enableAutomaticScroll() } else { - aiChatManager.disableAutomaticScroll() + chatHost.disableAutomaticScroll() } + onTranscriptScroll?.() } function submitSuggestion(suggestion: string) { - aiChatManager.sendRequest({ instructions: suggestion }) + chatHost.sendRequest({ instructions: suggestion }) } export function focusInput() { @@ -319,35 +340,31 @@ $effect(() => { if (aiChatInput) { - aiChatManager.setAiChatInput(aiChatInput) + chatHost.setAiChatInput(aiChatInput) } return () => { - aiChatManager.setAiChatInput(null) + chatHost.setAiChatInput(null) } }) // Also shown for a run held by another tab, labeled with where it is: the // dots say a turn is in flight even before the reader reaches the footer // note. Remote runs pause nothing and offer no Stop — this tab can't cancel. - const showTypingIndicator = $derived(aiChatManager.loading || aiChatManager.runHeldElsewhere) + const showTypingIndicator = $derived(chatHost.loading || chatHost.runHeldElsewhere) // The manual `@` context-picker button. Shown in SCRIPT/FLOW (workspace items + // code blocks) and APP (datatables, frontend files). Hidden in GLOBAL — there // `@`-context is still invoked inline by typing `@` in the input, so the button // is redundant. NAVIGATOR/ASK/API don't take @-context at all. const showContextPicker = $derived( - aiChatManager.mode === AIMode.SCRIPT || - aiChatManager.mode === AIMode.FLOW || - aiChatManager.mode === AIMode.APP + chatHost.mode === AIMode.SCRIPT || chatHost.mode === AIMode.FLOW || chatHost.mode === AIMode.APP ) - // File attachment is GLOBAL-mode only. - const canAttachFiles = $derived(aiChatManager.mode === AIMode.GLOBAL && !disabled) - // Steers the OS file picker toward text + image formats (soft hint; both attach - // to the message — text files after a content sniff). - const TEXT_FILE_ACCEPT = - 'image/*,text/*,.txt,.csv,.tsv,.json,.jsonl,.ndjson,.md,.markdown,.log,.yaml,.yml,.toml,.ini,.cfg,.conf,.env,.xml,.html,.htm,.css,.js,.mjs,.cjs,.ts,.tsx,.jsx,.py,.rb,.rs,.go,.java,.kt,.c,.h,.cpp,.cc,.cs,.php,.sh,.bash,.zsh,.sql,.svelte,.vue,.dockerfile' + const canAttachFiles = $derived(chatHost.supportsMessageAttachments && !disabled) + // Folders are linked as session-wide assets, which only a host that reads files in + // the browser can do — a host running the turn server-side takes attachments only. + const canLinkFolders = $derived(chatHost.supportsLinkedFolders && !disabled) let fileInputEl = $state(null) let folderInputEl = $state(null) let dragDepth = $state(0) @@ -373,12 +390,12 @@ } async function handleAddFiles(files: FileList | FileToAttach[]) { - const { added, rejected } = await aiChatManager.attachedFiles.addFiles(files) + const { added, rejected } = await chatHost.attachedFiles.addFiles(files) reportAddResult(added, rejected) } async function addDirHandle(dir: FileSystemDirectoryHandle) { - const { added, rejected } = await aiChatManager.attachedFiles.addFolder(dir) + const { added, rejected } = await chatHost.attachedFiles.addFolder(dir) reportAddResult(added, rejected) } @@ -471,8 +488,13 @@ const textFiles = looseFiles.filter((f) => !isImageFile(f)) if (textFiles.length > 0) await aiChatInput?.addTextFiles(textFiles) // Folders link as a live handle. - for (const h of handles.filter(isDirectoryHandle)) { - await addDirHandle(h) + const dirs = handles.filter(isDirectoryHandle) + if (dirs.length > 0 && !canLinkFolders) { + sendUserToast('Folders cannot be attached in this chat — drop individual files.', true) + } else { + for (const h of dirs) { + await addDirHandle(h) + } } } else { // Fallback (no File System Access API): snapshot dropped files AND folders by walking @@ -497,7 +519,10 @@ topLevelText.push(file) } } - if (folderEntries.length > 0) await handleAddFiles(folderEntries) + if (folderEntries.length > 0) { + if (canLinkFolders) await handleAddFiles(folderEntries) + else sendUserToast('Folders cannot be attached in this chat — drop individual files.', true) + } if (topLevelText.length > 0) await aiChatInput?.addTextFiles(topLevelText) } } @@ -524,9 +549,9 @@ input.value = '' } const autonomyAvailability = $derived({ - autoAcceptEditsAvailable: aiChatManager.autoAcceptEditsAvailable, - autoAcceptToolConfirmationsAvailable: aiChatManager.autoAcceptToolConfirmationsAvailable, - planModeAvailable: aiChatManager.planModeAvailable + autoAcceptEditsAvailable: chatHost.autoAcceptEditsAvailable, + autoAcceptToolConfirmationsAvailable: chatHost.autoAcceptToolConfirmationsAvailable, + planModeAvailable: chatHost.planModeAvailable }) const availableAutonomyModeOptions = $derived( autonomyModeOptions.filter((option) => option.isAvailable(autonomyAvailability)) @@ -534,8 +559,8 @@ // Fall back to ask-permission when the persisted mode isn't applicable in the // current AI mode (e.g. auto-accept edits while in a mode without edits). const effectiveAutonomyMode = $derived( - availableAutonomyModeOptions.some((option) => option.mode === aiChatManager.autonomyMode) - ? aiChatManager.autonomyMode + availableAutonomyModeOptions.some((option) => option.mode === chatHost.autonomyMode) + ? chatHost.autonomyMode : AIAutonomyMode.DEFAULT ) const showAutonomyModeSelector = $derived(!disabled && availableAutonomyModeOptions.length > 1) @@ -544,13 +569,13 @@ // The typing-dots indicator implies the AI is busy, which is misleading while // the loop is parked on the user; surface a text pill instead so users know to // act on the tool above. - const waitingForUserAction = $derived(aiChatManager.loading && !!pendingUserAction(messages)) + const waitingForUserAction = $derived(chatHost.loading && !!pendingUserAction(messages)) // Gated on `loading` because a card restored from history still looks parked: // its resolver left with the old page, so the composer must not advertise an // answer it cannot deliver. const pendingQuestionToolCallId = $derived.by(() => { - if (!aiChatManager.loading) { + if (!chatHost.loading) { return undefined } const pending = pendingUserActionDetail(messages) @@ -559,14 +584,14 @@ // Get app context for display when in APP mode const appContext = $derived.by((): SelectedContext | undefined => { - if (aiChatManager.mode !== AIMode.APP || !aiChatManager.appAiChatHelpers) { + if (chatHost.mode !== AIMode.APP || !chatHost.appAiChatHelpers) { return undefined } - return aiChatManager.appAiChatHelpers.getSelectedContext() + return chatHost.appAiChatHelpers.getSelectedContext() }) const yoloBypassedTools = $derived.by(() => { - return aiChatManager.tools + return chatHost.tools .filter((tool) => tool.requiresConfirmation === true || tool.bypassedByAutoAccept === true) .map((tool) => ({ name: tool.def.function.name, @@ -583,8 +608,7 @@ Math.max(0, yoloBypassedTools.length - visibleYoloBypassedTools.length) ) const showFlowPendingActionControls = $derived( - (aiChatManager.flowAiChatHelpers?.hasPendingChanges() ?? false) && - !aiChatManager.autoAcceptEditsActive + (chatHost.flowAiChatHelpers?.hasPendingChanges() ?? false) && !chatHost.autoAcceptEditsActive ) // A disabled state with no message (a remote hold, a spent free grant) keeps // the footer toolbar in place — swapping it for an empty strip would make @@ -592,11 +616,15 @@ // a real message (archived, AI off) still shows it, hold or not, matching // the precedence disabledMessage itself encodes. const footerMessageShown = $derived(disabled && disabledMessage !== '') + // `canAttachFiles` belongs in the group too: in GLOBAL mode the `+` always has the + // context picker or the autonomy selector beside it, but a host with attachments and + // nothing else would lose the group and the `+` with it. const showFooterLeftControls = $derived( !footerMessageShown && - (showContextPicker || + (canAttachFiles || + showContextPicker || showAutonomyModeSelector || - (aiChatManager.mode === AIMode.SCRIPT && hasDiff)) + (chatHost.mode === AIMode.SCRIPT && hasDiff)) ) @@ -694,12 +722,12 @@ the panel, or the Escape-to-stop focus check would wrongly reject them. --> {#each pastChats as chat (chat.id)}
{/if}
- {:else if aiChatManager.mode === AIMode.APP} + {:else if chatHost.mode === AIMode.APP} {#if showContext} {@render badgeRow()} {/if} @@ -1264,30 +1289,38 @@ {/if} {:else} -
- - {#if !bottomRightSnippet} -
- {@render sendStopButton()} -
- {/if} + +
+ {@render badgeRow()} + {@render imageChipsRow()} +
+ + {#if !bottomRightSnippet} +
+ {@render sendStopButton()} +
+ {/if} +
{/if} {#if bottomRightSnippet} diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index 85d70b1981..ab22a2ed7b 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -1,3 +1,4 @@ +import type { ChatViewHost } from './chatViewHost' import type { ScriptLang } from '$lib/gen/types.gen' import { JobService, type CompletedJob } from '$lib/gen' import type { FlowOptions, ScriptOptions } from './ContextManager.svelte' @@ -445,9 +446,27 @@ function planModeHostFor(m: AIChatManager): PlanModeHost { } } -export class AIChatManager { +export class AIChatManager implements ChatViewHost { contextManager = new ContextManager() historyManager = new HistoryManager() + // The copilot owns its model choice and its own transcript, so both chat + // affordances apply here. See ChatViewHost for hosts where they don't. + supportsModelSettings = true + supportsMessageEditing = true + // The copilot turn is the attachments themselves when there is no text. + requiresMessageText = false + // Attachments and linked folders are GLOBAL-mode affordances. Declared as + // getters because `mode` changes under a mounted composer. + get supportsMessageAttachments() { + return this.mode === AIMode.GLOBAL + } + get supportsLinkedFolders() { + return this.mode === AIMode.GLOBAL + } + // Steers the OS file picker toward text + image formats (a soft hint; both attach to + // the message — text files after a content sniff). + attachmentAccept = + 'image/*,text/*,.txt,.csv,.tsv,.json,.jsonl,.ndjson,.md,.markdown,.log,.yaml,.yml,.toml,.ini,.cfg,.conf,.env,.xml,.html,.htm,.css,.js,.mjs,.cjs,.ts,.tsx,.jsx,.py,.rb,.rs,.go,.java,.kt,.c,.h,.cpp,.cc,.cs,.php,.sh,.bash,.zsh,.sql,.svelte,.vue,.dockerfile' /** Files the user attached to the current GLOBAL-mode conversation. */ attachedFiles = new AttachedFilesStore() /** Markdown artifacts the copilot created for the current session. */ diff --git a/frontend/src/lib/components/copilot/chat/AIChatMessage.svelte b/frontend/src/lib/components/copilot/chat/AIChatMessage.svelte index 12553be961..afcffbb400 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatMessage.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChatMessage.svelte @@ -3,7 +3,7 @@ import type { DisplayMessage, ToolDisplayMessage } from './shared' import ContextElementBadge from './ContextElementBadge.svelte' import AssistantMessage from './AssistantMessage.svelte' - import { getAiChatManager } from './aiChatManagerContext' + import { getChatViewHost } from './chatViewHost' import { Button } from '$lib/components/common' import { RefreshCwIcon, Undo2Icon } from 'lucide-svelte' import AIChatInput from './AIChatInput.svelte' @@ -15,7 +15,7 @@ import ExpandableImage from '$lib/components/common/image/ExpandableImage.svelte' import { workspaceStore } from '$lib/stores' - const aiChatManager = getAiChatManager() + const chatHost = getChatViewHost() // Paths in a message name items the chat's tools reach, so they resolve against the // operating workspace, never `workspaceStore`: a fork session leaves the store on the @@ -25,7 +25,7 @@ // Registers the dependency that `operatingWorkspace`'s own untracked // `get(workspaceStore)` cannot. void $workspaceStore - return aiChatManager.operatingWorkspace + return chatHost.operatingWorkspace }) // Per-message expand/collapse state for paste chips shown in the bubble. @@ -62,7 +62,12 @@ let editContext = $state([]) function editMessage() { - if (message.role !== 'user' || editingMessageIndex !== null || aiChatManager.loading) { + if ( + !chatHost.supportsMessageEditing || + message.role !== 'user' || + editingMessageIndex !== null || + chatHost.loading + ) { return } editContext = [...(message.contextElements ?? [])] @@ -79,7 +84,9 @@ message.role === 'tool' && 'mb-1', message.role === 'user' && messageIndex > 0 && 'mt-4 mb-6', isLast && '!mb-12', - message.role !== 'user' ? 'cursor-default' : 'cursor-pointer' + message.role !== 'user' || !chatHost.supportsMessageEditing + ? 'cursor-default' + : 'cursor-pointer' )} role="button" tabindex="0" @@ -116,7 +123,7 @@ bind:selectedContext={editContext} initialInstructions={message.content} initialPastes={message.pastes} - initialImages={aiChatManager.storedImages(messageIndex)} + initialImages={chatHost.storedImages(messageIndex)} initialFiles={message.files} {editingMessageIndex} onClickOutside={() => (editingMessageIndex = null)} @@ -185,9 +192,9 @@ on:click={() => { if (message.snapshot) { if (message.snapshot.type === 'flow') { - aiChatManager.flowAiChatHelpers?.revertToSnapshot(message.snapshot.value) + chatHost.flowAiChatHelpers?.revertToSnapshot(message.snapshot.value) } else if (message.snapshot.type === 'app') { - aiChatManager.appAiChatHelpers?.revertToSnapshot(message.snapshot.value) + chatHost.appAiChatHelpers?.revertToSnapshot(message.snapshot.value) } } }} @@ -206,7 +213,7 @@ variant="default" title="Retry generation" startIcon={{ icon: RefreshCwIcon }} - onclick={() => aiChatManager.retryRequest(messageIndex)} + onclick={() => chatHost.retryRequest(messageIndex)} > Retry diff --git a/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte b/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte index 9dd94fa4ea..914ad51bc9 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte @@ -1,10 +1,12 @@ {#snippet externalLinkIcon()} {/snippet} - - {#snippet buttonReplacement()} -
- -
- {/snippet} - {#snippet menu({ item, builders, close })} -
- - {#if promptSettings} - - {/if} + -
-
Model
-
- {#each models as m (m.provider + m.model)} - selectModel(m)} - > - {m.model} - {#if m.model === providerModel.model && m.provider === providerModel.provider} - - {/if} - - {/each} -
- -
- {#if capability.supported} - - -
- Thinking - {currentStop} -
- {#if stops.length > 1} - -
- selectReasoning(stops[+e.currentTarget.value])} - use:isolatePointer - class="lean-range no-default-style w-full" - aria-label="Reasoning effort" - /> -
- {/if} - - {:else} - -
-
Thinking
-
Not supported by this model
-
- {/if} - - - (thinkingPreferences.expandByDefault = !thinkingPreferences.expandByDefault)} - > - Always expand thinking - {#if thinkingPreferences.expandByDefault} - - {/if} - -
- {/snippet} - - - {#if promptSettings} {/if} - - diff --git a/frontend/src/lib/components/copilot/chat/AskUserQuestionDisplay.svelte b/frontend/src/lib/components/copilot/chat/AskUserQuestionDisplay.svelte index 403b42864d..17cc2bb4c1 100644 --- a/frontend/src/lib/components/copilot/chat/AskUserQuestionDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/AskUserQuestionDisplay.svelte @@ -4,7 +4,7 @@ import { CircleHelp, ArrowUp, Plus, Square, SquareCheck } from 'lucide-svelte' import Button from '$lib/components/common/button/Button.svelte' import TextInput from '$lib/components/text_input/TextInput.svelte' - import { getAiChatManager } from './aiChatManagerContext' + import { getChatViewHost } from './chatViewHost' import type { UserQuestionDisplay } from './shared' // Sessions inject a per-pane `AIChatManager` via context; outside of @@ -12,7 +12,7 @@ // this, answers clicked inside a session would dispatch to the singleton's // pending callbacks map (which doesn't have the session manager's question // callback), and the AI loop would stall. - const aiChatManager = getAiChatManager() + const chatHost = getChatViewHost() interface Props { toolCallId: string @@ -93,14 +93,14 @@ } return } - aiChatManager.handleUserQuestionAnswer(toolCallId, [choice]) + chatHost.handleUserQuestionAnswer(toolCallId, [choice]) } function submitPicked() { if (!multiSelect || picked.size === 0) { return } - aiChatManager.handleUserQuestionAnswer(toolCallId, [...picked]) + chatHost.handleUserQuestionAnswer(toolCallId, [...picked]) } function submitCustomAnswer() { @@ -119,7 +119,7 @@ return } - aiChatManager.handleUserQuestionAnswer(toolCallId, [answer]) + chatHost.handleUserQuestionAnswer(toolCallId, [answer]) } function handleChoiceKeydown(event: KeyboardEvent, choice: string, index: number) { diff --git a/frontend/src/lib/components/copilot/chat/AssistantMessage.svelte b/frontend/src/lib/components/copilot/chat/AssistantMessage.svelte index f06941c2a4..44cedf323c 100644 --- a/frontend/src/lib/components/copilot/chat/AssistantMessage.svelte +++ b/frontend/src/lib/components/copilot/chat/AssistantMessage.svelte @@ -12,6 +12,7 @@ workspaceItemRegistry } from './workspaceItems.svelte' import { markdownProse } from '$lib/components/markdownProse' + import DisplayResult from '$lib/components/DisplayResult.svelte' interface Props { message: DisplayMessage @@ -60,6 +61,20 @@ return rest === 0 ? `${minutes}m` : `${minutes}m ${rest}s` } + const stepName = $derived(message.role === 'assistant' ? message.stepName : undefined) + + // A flow step can return a file rather than text; the raw JSON would be + // unreadable, so hand it to the result viewer instead of the markdown renderer. + const s3Object = $derived.by(() => { + if (!message.content.startsWith('{')) return undefined + try { + const parsed = JSON.parse(message.content) + return parsed?.type === 'windmill_s3_object' && parsed?.s3 ? parsed : undefined + } catch { + return undefined + } + }) + const candidatePaths = $derived(extractCandidatePaths(message.content)) const rendererPlugin = { renderer: { @@ -98,6 +113,12 @@ }) +{#if stepName} +
+ {stepName} +
+{/if} + {#if reasoning} {/if} -{#if message.content} +{#if s3Object} + +{:else if message.content}
diff --git a/frontend/src/lib/components/copilot/chat/ContextTextarea.svelte b/frontend/src/lib/components/copilot/chat/ContextTextarea.svelte index 8ae3b51b56..976b969b3f 100644 --- a/frontend/src/lib/components/copilot/chat/ContextTextarea.svelte +++ b/frontend/src/lib/components/copilot/chat/ContextTextarea.svelte @@ -1,4 +1,5 @@ - - -
+
{@render leading?.()} @@ -830,11 +817,7 @@ {placeholder} class={twMerge( 'textarea-input resize-none caret-black dark:caret-white overflow-clip', - // The box (border/ring) lives on the wrapper; kill the textarea's own - // @tailwindcss/forms border, focus ring, and background so only the - // wrapper reads as the field. - '!border-transparent !bg-transparent !shadow-none focus:!border-transparent focus:!ring-0', - 'disabled:cursor-not-allowed disabled:placeholder:text-disabled', + COMPOSER_FIELD_RESET, CHAT_INPUT_PADDING, className )} diff --git a/frontend/src/lib/components/copilot/chat/ContextUsageIndicator.svelte b/frontend/src/lib/components/copilot/chat/ContextUsageIndicator.svelte index 96b452120b..cd6c58b2a3 100644 --- a/frontend/src/lib/components/copilot/chat/ContextUsageIndicator.svelte +++ b/frontend/src/lib/components/copilot/chat/ContextUsageIndicator.svelte @@ -1,16 +1,16 @@ -{#if aiChatManager.queuedMessage || aiChatManager.queuedImages.length > 0 || aiChatManager.queuedFiles.length > 0 || (aiChatManager.queuedContext?.length ?? 0) > 0} +{#if chatHost.queuedMessage || chatHost.queuedImages.length > 0 || chatHost.queuedFiles.length > 0 || (chatHost.queuedContext?.length ?? 0) > 0}
- {#if aiChatManager.queuedMessage || aiChatManager.queuedImages.length > 0 || aiChatManager.queuedFiles.length > 0} + {#if chatHost.queuedMessage || chatHost.queuedImages.length > 0 || chatHost.queuedFiles.length > 0} - {:else if aiChatManager.queuedContext?.length} + {:else if chatHost.queuedContext?.length}
- {#each aiChatManager.queuedContext as element (contextElementKey(element))} + {#each chatHost.queuedContext as element (contextElementKey(element))} {/each}
@@ -82,7 +82,7 @@ iconOnly title="Remove queued message and put it back in the input" startIcon={{ icon: X }} - on:click={() => aiChatManager.dequeueMessage()} + on:click={() => chatHost.dequeueMessage()} />
{/if} diff --git a/frontend/src/lib/components/copilot/chat/ToolConfirmationFooter.svelte b/frontend/src/lib/components/copilot/chat/ToolConfirmationFooter.svelte index 699b79a292..fcf943cb34 100644 --- a/frontend/src/lib/components/copilot/chat/ToolConfirmationFooter.svelte +++ b/frontend/src/lib/components/copilot/chat/ToolConfirmationFooter.svelte @@ -1,7 +1,7 @@ diff --git a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte index 620357ff87..a0d16043a4 100644 --- a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte @@ -21,9 +21,9 @@ } from './planMode' import { Button } from '$lib/components/common' import { markdownProse } from '$lib/components/markdownProse' - import { getAiChatManager } from './aiChatManagerContext' + import { getChatViewHost } from './chatViewHost' - const aiChatManager = getAiChatManager() + const chatHost = getChatViewHost() import { isActiveUserQuestion, type ToolDisplayMessage } from './shared' import ChatCollapsibleCard from './ChatCollapsibleCard.svelte' import { twMerge } from 'tailwind-merge' @@ -69,7 +69,7 @@ const planLabel = $derived((planState && planCopy?.[planState]) ?? '') const planDoc = $derived( message.planArtifactId - ? aiChatManager.artifacts.artifacts.find((a) => a.id === message.planArtifactId) + ? chatHost.artifacts.artifacts.find((a) => a.id === message.planArtifactId) : undefined ) // The version this card wrote, not the document's current one, since later proposals move it on. @@ -201,7 +201,7 @@ title="Open this plan in the side panel: {planDoc.name}" startIcon={{ icon: FileText, classes: PLAN_MODE_TEXT_COLOR }} endIcon={{ icon: PanelRight }} - on:click={() => aiChatManager.openArtifact?.(planDoc.id, planDoc.name, planCardVersion)} + on:click={() => chatHost.openArtifact?.(planDoc.id, planDoc.name, planCardVersion)} > Plan diff --git a/frontend/src/lib/components/copilot/chat/chatViewHost.ts b/frontend/src/lib/components/copilot/chat/chatViewHost.ts new file mode 100644 index 0000000000..9e3bdb44ac --- /dev/null +++ b/frontend/src/lib/components/copilot/chat/chatViewHost.ts @@ -0,0 +1,152 @@ +import { getContext, setContext } from 'svelte' +import type { AIMode, AIAutonomyMode } from './AIChatManager.svelte' +import { getAiChatManager } from './aiChatManagerContext' +import type { DisplayMessage, Tool } from './shared' +import type { ContextElement } from './context' +import type { AttachedImage } from './imageUtils' +import type { AttachedTextFile } from './textFileUtils' +import type { PasteAttachment } from './pasteTokens' +import type { AttachedFilesStore } from './files/attachedFiles.svelte' +import type { SessionArtifactsStore } from './artifacts/artifactsState.svelte' +import type { ArtifactVersionTarget } from '$lib/components/sessions/previewRouter' +import type { FlowAIChatHelpers } from './flow/core' +import type { AppAIChatHelpers } from './app/core' +import type AIChatInput from './AIChatInput.svelte' + +export type ChatSendRequestOptions = { + instructions?: string + pastes?: PasteAttachment[] + images?: AttachedImage[] + files?: AttachedTextFile[] + /** Selected-context snapshot for this turn, in place of the live selection. Set + * whenever a send settles its context ahead of the turn. A host with no context + * of its own ignores it. */ + contextOverride?: ContextElement[] + /** Where `contextOverride` came from. 'pinned': chips picked for THIS message, so + * they are consumed from the live selection on send. 'replay': an edit or retry + * resending an older message's context, already consumed long ago. */ + contextOverrideOrigin?: 'pinned' | 'replay' +} + +/** + * What the chat view components (AIChatDisplay and everything it renders) need + * from whatever is driving the conversation. AIChatManager implements it for the + * copilot's own LLM loop; FlowChatViewHost implements it over a flow run's + * conversation so both chats render through the same components. + * + * Each affordance is gated by the field that answers for it — attachments by + * `supportsMessageAttachments`, the model button by `supportsModelSettings`, and so on — + * so a host turns on exactly what it can serve, and a new one is a matter of answering + * these fields rather than of being a copilot. `mode` is the exception, still read + * directly for chrome that only the copilot has. + */ +export interface ChatViewHost { + // Transcript + displayMessages: DisplayMessage[] + /** API-level messages. Only the count is read (context usage visibility). */ + messages: readonly unknown[] + contextTokens: number + /** The workspace a message's paths and jobs resolve against, which a fork session + * pins away from the navigated one. */ + readonly operatingWorkspace: string | undefined + loading: boolean + /** A turn this tab can neither follow nor stop, held by another tab on the same chat. */ + readonly runHeldElsewhere: boolean + loadingLabel: string | undefined + compacting: boolean + currentReply: string + currentReasoning: string + currentReasoningActive: boolean + readonly reasoningHiddenIndicatorLabel: string | undefined + readonly automaticScroll: boolean + enableAutomaticScroll: () => void + disableAutomaticScroll: () => void + + // Composer + instructions: string + readonly sendInFlight: boolean + /** Resolves to whether the draft was consumed as a turn. */ + sendRequest: (options?: ChatSendRequestOptions) => Promise + cancel: (reason?: string) => void + setAiChatInput: (aiChatInput: AIChatInput | null) => void + readonly queuedMessage: string + queuedContext: ContextElement[] | undefined + readonly queuedImages: AttachedImage[] + readonly queuedFiles: AttachedTextFile[] + queueMessage: ( + text: string, + images?: AttachedImage[], + context?: ContextElement[], + files?: AttachedTextFile[] + ) => void + dequeueMessage: () => void + setComposerStaged: (key: string, editingIndex: number | null, bytes: number) => void + clearComposerStaged: (key: string) => void + attachmentBytesExcluding: (selfKey: string) => number + + // Per-message actions + storedImages: (displayMessageIndex: number) => AttachedImage[] | undefined + retryRequest: (messageIndex: number) => void + restartGeneration: ( + displayMessageIndex: number, + newContent?: string, + pastes?: PasteAttachment[], + images?: AttachedImage[], + editedContext?: ContextElement[], + files?: AttachedTextFile[] + ) => void | Promise + handleUserQuestionAnswer: (toolId: string, choices: string[]) => boolean + handleToolConfirmation: (toolId: string, confirmed: boolean) => void + /** A tool is waiting on a run form the user is filling in. Escape belongs to that form + * then, not to the turn — see AIChatDisplay's window handler. */ + readonly hasPendingRunForm: boolean + isRunFormPending: (toolCallId: string) => boolean + + // Copilot-only surfaces. Left undefined/false by hosts that have no LLM loop + // of their own; the chrome they drive hides itself. + mode?: AIMode + isSessionChat: boolean + /** Model + reasoning picker. Off where the model is configured elsewhere. */ + supportsModelSettings: boolean + /** Click a user message to edit and resend it. Needs a host that can rewind + * its own transcript, which a host replaying a server-side run cannot. */ + supportsMessageEditing: boolean + /** The `+` menu's file entry and drag-and-drop onto the panel. */ + supportsMessageAttachments: boolean + /** The turn needs text: attachments alone cannot be sent. True where the consumer + * requires a message of its own — an AI agent step refuses a run with neither a + * `user_message` nor manual memory. */ + requiresMessageText: boolean + /** The `+` menu's folder entries, backed by `attachedFiles`. A linked folder is a + * live handle on the user's disk, so only a host reading files in the browser has one. */ + supportsLinkedFolders: boolean + /** `accept` for the file picker. */ + attachmentAccept: string + tools: Tool[] + autonomyMode: AIAutonomyMode + setAutonomyMode: (mode: AIAutonomyMode) => void + readonly autoAcceptEditsActive: boolean + readonly autoAcceptEditsAvailable: boolean + readonly autoAcceptToolConfirmationsAvailable: boolean + readonly planModeAvailable: boolean + attachedFiles: AttachedFilesStore + artifacts: SessionArtifactsStore + openArtifact?: (artifactId: string, name: string, version?: ArtifactVersionTarget) => void + flowAiChatHelpers?: FlowAIChatHelpers + appAiChatHelpers?: AppAIChatHelpers +} + +const CHAT_VIEW_HOST_CONTEXT_KEY = 'chatViewHost' + +export function setChatViewHost(host: ChatViewHost) { + setContext(CHAT_VIEW_HOST_CONTEXT_KEY, host) +} + +/** + * Resolve the host driving the chat in this subtree. Falls back to the + * AIChatManager (scoped instance or app-wide singleton) so every existing + * copilot chat keeps working without setting anything. + */ +export function getChatViewHost(): ChatViewHost { + return getContext(CHAT_VIEW_HOST_CONTEXT_KEY) ?? getAiChatManager() +} diff --git a/frontend/src/lib/components/copilot/chat/composerBox.ts b/frontend/src/lib/components/copilot/chat/composerBox.ts new file mode 100644 index 0000000000..f85783d988 --- /dev/null +++ b/frontend/src/lib/components/copilot/chat/composerBox.ts @@ -0,0 +1,27 @@ +/** + * The composer's box, shared by both of AIChatInput's branches — the rich + * ContextTextarea and the plain textarea a host without @-context gets. + * + * Border and rounding live on the WRAPPER, never on the field, so the chip rows + * (context, files, images) sit inside the box above the text. The field's own + * @tailwindcss/forms border, ring and background are neutralised so only the + * wrapper reads as the input. + * + * The disabled treatment is on the wrapper for the same reason: `disabled` on the + * field alone leaves it looking exactly like a usable one, so the only cue that + * typing is refused is placeholder text the eye reads as an invitation. + */ + +const BOX_BASE = 'w-full scroll-pb-2 rounded-md border border-border-light transition-colors' + +export function composerBoxClass(disabled: boolean = false): string { + return `${BOX_BASE} ${ + disabled + ? 'bg-surface-disabled cursor-not-allowed' + : 'bg-surface-input focus-within:border-border-selected' + }` +} + +/** Applied to the field inside the box; without it the field draws a second border. */ +export const COMPOSER_FIELD_RESET = + '!border-transparent !bg-transparent !shadow-none focus:!border-transparent focus:!ring-0 disabled:cursor-not-allowed disabled:placeholder:text-disabled' 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 0b1ea0952e..a78f09071a 100644 --- a/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.test.ts @@ -31,6 +31,18 @@ const CATALOG = [ properties: { page: { type: 'integer' }, per_page: { type: 'integer' } } } }, + { + name: 'getJobUpdates', + description: 'Get job updates', + instructions: '', + path: '/w/{workspace}/jobs_u/getupdate/{id}', + method: 'GET', + path_params_schema: { + type: 'object', + properties: { workspace: { type: 'string' }, id: { type: 'string' } }, + required: ['workspace', 'id'] + } + }, { name: 'getJob', description: 'Get job details', @@ -195,6 +207,9 @@ describe('call_api_get', () => { const mutating = await run('call_api_get', { name: 'cancelQueuedJob' }) expect(mutating.error).toContain('call_api_endpoint') + const job = await run('call_api_get', { name: 'getJob' }) + expect(job.error).toContain('get_run') + const deleting = await run('call_api_endpoint', { name: 'deleteSchedule' }) expect(deleting.error).toContain('delete_workspace_item') @@ -248,7 +263,7 @@ describe('call_api_get', () => { }) it('returns the endpoint schema when a required path param is missing', async () => { - const result = await run('call_api_get', { name: 'getJob' }) + const result = await run('call_api_get', { name: 'getJobUpdates' }) expect(result.success).toBe(false) expect(result.error).toContain('id') expect(result.schema.path_params_schema.required).toContain('id') diff --git a/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts b/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts index 42d45916eb..308d222544 100644 --- a/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts +++ b/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts @@ -58,7 +58,8 @@ const COVERED_ENDPOINTS: Record = { searchDocs: 'search_docs', readDocsPage: 'read_docs_page', listJobs: 'list_runs', - getJobLogs: 'get_job_logs', + getJob: 'get_run', + getJobLogs: 'get_run', runScriptPreviewAndWaitResult: 'test_run_script' } 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 051304cafa..fe0477a544 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -85,10 +85,33 @@ vi.mock('$lib/gen', async () => { runScriptByPath: vi.fn(async () => 'job-script-by-path'), getJob: vi.fn(async () => ({ type: 'CompletedJob', + id: 'job-123', + job_kind: 'script', + script_path: 'f/team/runner', success: true, + canceled: false, + args: { n: 3 }, result: { ok: true }, logs: 'test logs' })), + getJobArgs: vi.fn(async () => ({ big: 'real args' })), + getCompletedJobResultMaybe: vi.fn(async () => ({ completed: true, result: { big: 'x' } })), + getFlowAllResults: vi.fn(async () => ({ + entries: [ + { + job_id: 'job-123', + label: 'Flow', + kind: 'script', + depth: 0, + sibling_index: 1, + sibling_count: 1, + status: 'success', + success: true + } + ], + truncated: false, + scope_filtered: false + })), // What every job wait polls first; unmocked it reaches the real client and the // wait never returns. Answers completed, so one tick settles the job. getJobUpdates: vi.fn(async () => ({ @@ -489,7 +512,7 @@ describe('global AI tools', () => { expect(names).toContain('test_run_script') expect(names).toContain('test_run_flow') expect(names).toContain('test_run_step') - expect(names).toContain('get_job_logs') + expect(names).toContain('get_run') expect(names).toContain('list_runs') }) @@ -715,29 +738,210 @@ describe('global AI tools', () => { ) }) - it('fetches job logs by id and always suppresses the backend ansi hint line', async () => { - const result = await callGlobalTool('get_job_logs', { id: 'job-123' }) + it('returns args, result and logs of a run in one call', async () => { + const result = await callGlobalTool('get_run', { id: 'job-123' }) expect(JobService.getJobLogs).toHaveBeenCalledWith({ workspace: WORKSPACE, id: 'job-123', + // The backend's "to remove ansi colors, use: sed ..." hint is noise for + // the model, so it is always suppressed. removeAnsiWarnings: true }) - expect(result).toBe('job log line 1\njob log line 2') - // The logs must be surfaced as the tool result so the details panel shows - // them rather than "No result yet". + const parsed = JSON.parse(result) + expect(parsed.run).toMatchObject({ + status: 'success', + path: 'f/team/runner', + args: expect.stringContaining('"n": 3'), + result: expect.stringContaining('"ok": true'), + logs: 'job log line 1\njob log line 2' + }) + // The result must be surfaced as the tool result so the details panel shows + // it rather than "No result yet". expect(toolCallbacks.setToolStatus).toHaveBeenCalledWith( - 'test-get_job_logs', - expect.objectContaining({ result: 'job log line 1\njob log line 2' }) + 'test-get_run', + expect.objectContaining({ result }) ) }) - it('reports when a job has no logs', async () => { + it('reports when a run has no logs, and tells that apart from logs it could not read', async () => { vi.mocked(JobService.getJobLogs).mockResolvedValueOnce(' ') + expect(JSON.parse(await callGlobalTool('get_run', { id: 'job-empty' })).run.logs).toBe( + 'No logs for this run.' + ) - const result = await callGlobalTool('get_job_logs', { id: 'job-empty' }) + // A failed fetch must not read as "this run logged nothing" — the model + // would report that to the user as fact. + vi.mocked(JobService.getJobLogs).mockRejectedValueOnce(new Error('boom')) + expect(JSON.parse(await callGlobalTool('get_run', { id: 'job-123' })).run.logs).toBe( + 'Logs could not be read for this run.' + ) - expect(result).toBe('No logs available for this job.') + // Nor does every failed read reject: the generated client resolves undefined + // when it cannot read the body, which lands on the same "no logs" branch. + vi.mocked(JobService.getJobLogs).mockResolvedValueOnce(undefined as any) + expect(JSON.parse(await callGlobalTool('get_run', { id: 'job-123' })).run.logs).toBe( + 'Logs could not be read for this run.' + ) + }) + + it('keeps the end of a long log, and never opens it on half a surrogate pair', async () => { + // 12002 code points over 24003 UTF-16 units: the 12000-unit tail opens one + // unit into a unicorn, so the lone low surrogate has to be dropped. + vi.mocked(JobService.getJobLogs).mockResolvedValueOnce('🦄'.repeat(12001) + 'z') + + const [note, body] = JSON.parse( + await callGlobalTool('get_run', { id: 'job-123' }) + ).run.logs.split('\n') + + // The note goes first: the tail is what the model came for, and a note at + // the end would read as the last thing the run logged. + expect(note).toContain('12002 chars total') + expect(body).toHaveLength(11999) + expect(body.codePointAt(0)).toBe(0x1f984) + expect(body.endsWith('🦄z')).toBe(true) + }) + + it('keeps the step tree optional: a failed tree fetch still returns the run itself', async () => { + vi.mocked(JobService.getFlowAllResults).mockRejectedValueOnce(new Error('tree unavailable')) + + const parsed = JSON.parse(await callGlobalTool('get_run', { id: 'job-123' })) + + expect(parsed.run).toMatchObject({ args: expect.stringContaining('"n": 3') }) + expect(parsed.run.logs).toBe('job log line 1\njob log line 2') + // Silence here would read as "this flow ran no steps", which the model + // would then report to the user as fact. + expect(parsed.run.steps_unavailable).toBe(true) + // The tree's root entry normally names the run; without it nothing does. + expect(parsed.run.job_id).toBe('job-123') + + // And the tree read fails the same two ways the log read does: reading + // `.entries` off a resolved undefined throws, which would cost the model the + // job and logs already in hand rather than just the tree. + vi.mocked(JobService.getFlowAllResults).mockResolvedValueOnce(undefined as any) + const noTree = JSON.parse(await callGlobalTool('get_run', { id: 'job-123' })) + expect(noTree.run.steps_unavailable).toBe(true) + expect(noTree.run.logs).toBe('job log line 1\njob log line 2') + }) + + it('reports why a run was canceled or died, the fields getJob used to carry', async () => { + vi.mocked(JobService.getJob).mockResolvedValueOnce({ + type: 'CompletedJob', + id: 'job-oom', + job_kind: 'script', + success: false, + canceled: true, + canceled_by: 'alice', + canceled_reason: 'exceeded memory limit', + mem_peak: 2097152 + } as any) + + const run = JSON.parse(await callGlobalTool('get_run', { id: 'job-oom' })).run + + expect(run.status).toBe('canceled') + expect(run.canceled_by).toBe('alice') + expect(run.canceled_reason).toBe('exceeded memory limit') + expect(run.mem_peak_kb).toBe(2097152) + }) + + // Over ~90KB the job endpoint elides the payload, leaving the step tree as the + // only place a real (server-side truncated) head of the result survives. + function mockElidedJob() { + vi.mocked(JobService.getJob).mockResolvedValueOnce({ + type: 'CompletedJob', + id: 'job-big', + job_kind: 'script', + success: true, + canceled: false, + args: { reason: 'WINDMILL_TOO_BIG' }, + result: 'WINDMILL_TOO_BIG' + } as any) + vi.mocked(JobService.getFlowAllResults).mockResolvedValueOnce({ + entries: [ + { + job_id: 'job-big', + label: 'Flow', + kind: 'script', + depth: 0, + sibling_index: 1, + sibling_count: 1, + status: 'success', + success: true, + result_prefix: '{"blob":"real head"}', + result_length: 300018 + } + ], + truncated: false, + scope_filtered: false + } as any) + } + + it("reports getJob's WINDMILL_TOO_BIG placeholders instead of fetching around them", async () => { + mockElidedJob() + + const run = JSON.parse(await callGlobalTool('get_run', { id: 'job-big' })).run + + // The marker is never the run's own value, so it must not reach the model. + expect(JSON.stringify(run)).not.toContain('WINDMILL_TOO_BIG') + // The result falls to the step tree's real server-side head, flagged and + // sized so the model can't mistake the fragment for the whole payload. + expect(run.result).toBe('{"blob":"real head"}') + expect(run.result_total_chars).toBe(300018) + expect(run.result_truncated).toBe(true) + expect(run.args_truncated).toBe(true) + expect(run.args).toBeUndefined() + // The endpoints that return these whole take no length parameter, so + // reaching for one would pull the entire payload into the tab. + expect(JobService.getJobArgs).not.toHaveBeenCalled() + expect(JobService.getCompletedJobResultMaybe).not.toHaveBeenCalled() + }) + + it('keeps a payload that merely carries the marker string in a reason of its own', async () => { + // The backend elides a result to the bare string and args to exactly + // {reason: marker}. A payload with that reason plus fields of its own is the + // run's own value, and withholding it would report an elision that never was. + vi.mocked(JobService.getJob).mockResolvedValueOnce({ + type: 'CompletedJob', + id: 'job-reason', + job_kind: 'script', + success: false, + canceled: false, + args: { reason: 'WINDMILL_TOO_BIG', retries: 2 }, + result: { reason: 'WINDMILL_TOO_BIG', code: 42 } + } as any) + + const run = JSON.parse(await callGlobalTool('get_run', { id: 'job-reason' })).run + + expect(run.args_truncated).toBeUndefined() + expect(run.result_truncated).toBeUndefined() + expect(run.args).toContain('"retries": 2') + expect(run.result).toContain('"code": 42') + }) + + it('reports skipped and suspended runs as such rather than success or running', async () => { + // `success` is true for a skipped job, and a suspended job is `running`. + vi.mocked(JobService.getJob).mockResolvedValueOnce({ + type: 'CompletedJob', + id: 'job-skipped', + job_kind: 'script', + success: true, + canceled: false, + is_skipped: true + } as any) + expect(JSON.parse(await callGlobalTool('get_run', { id: 'job-skipped' })).run.status).toBe( + 'skipped' + ) + + vi.mocked(JobService.getJob).mockResolvedValueOnce({ + type: 'QueuedJob', + id: 'job-suspended', + job_kind: 'flow', + running: true, + suspend: 1 + } as any) + expect(JSON.parse(await callGlobalTool('get_run', { id: 'job-suspended' })).run.status).toBe( + 'suspended' + ) }) it('searches hub scripts without fetching script contents', async () => { @@ -6302,7 +6506,7 @@ describe('prepareGlobalSystemMessage', () => { it('dispatches to the registered handler with the session id and default limit of 20', async () => { const callbacks: ToolCallbacks = { setToolStatus: vi.fn(), removeToolStatus: vi.fn() } const handler = vi.fn(() => ({ - aiResult: 'runs output. Next step: call get_job_logs.', + aiResult: 'runs output. Next step: call get_run.', uiMessage: 'Listed 1 app run', toolResult: '[{"job_id":"job-1","component":"backend.1","status":"completed","created_at":1718000000000,"started_at":1718000000000,"duration_ms":1000}]' @@ -6311,7 +6515,7 @@ describe('prepareGlobalSystemMessage', () => { const result = await callGlobalTool('list_app_runs', {}, callbacks, { sessionId: 'sess-runs' }) - expect(result).toBe('runs output. Next step: call get_job_logs.') + expect(result).toBe('runs output. Next step: call get_run.') expect(handler).toHaveBeenCalledWith({ sessionId: 'sess-runs', limit: 20 }) expect(callbacks.setToolStatus).toHaveBeenLastCalledWith('test-list_app_runs', { content: 'Listed 1 app run', diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index d34085f317..19d9768d49 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -30,7 +30,6 @@ import type { Flow, FlowModule, FlowValue, - Job, ListableApp, ListableResource, ListableVariable, @@ -187,7 +186,7 @@ import { getDraftDiffValues } from '$lib/utils_draft_deploy' import { changedLineIndices, draftDeployedPatch, windowPatch } from './draftDiff' -import { getFlowRunDetails } from './flowRunTree' +import { getRun, summarizeRun } from './flowRunTree' import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' import { invalidateWorkspaceComparison } from '$lib/workspaceComparison' import type { UserDraftItemKind } from '$lib/gen' @@ -693,17 +692,13 @@ const searchResourceTypesSchema = z.object({ .describe('Max number of resource types to return. Defaults to 5.') }) -const getJobLogsSchema = z.object({ - id: z.string().describe('The UUID of the job to fetch logs for.') -}) - -const getFlowRunDetailsSchema = z.object({ - id: z.string().describe('The UUID of the flow run to inspect.'), +const getRunSchema = z.object({ + id: z.string().describe('The UUID of the run (job) to inspect.'), step: z .string() .optional() .describe( - 'Step to drill into for its result (returned in full up to 12k chars), addressed by the step ids shown in the tree: "b" for a top-level step, "b/c" for a step inside a subflow, "b[12]" for iteration 12 of a loop or attempt 12 of a retried step (1-based), composable as "b[12]/c". Omit to get the whole per-step tree.' + 'Step to drill into for its result (returned in full up to 12k chars), addressed by the step ids shown in the tree: "b" for a top-level step, "b/c" for a step inside a subflow, "b[12]" for iteration 12 of a loop or attempt 12 of a retried step (1-based), composable as "b[12]/c". Omit to get the run itself.' ) }) @@ -1356,8 +1351,8 @@ Rules: ${pipelineBullet} - After creating or editing a script or flow draft, run test_run_script, test_run_flow, or test_run_step with representative args before reporting that it works. These tools prefer drafts, so testing does not require deployment. - Do the same for a raw app: run test_run_app_runnable on each backend runnable you wrote or changed before saying the app works. A bundle that compiles proves nothing about whether the runnables run. An inline runnable executes the app's draft code; a path runnable executes the DEPLOYED script/flow it names, so a path runnable aimed at something you have not deployed fails here — that failure is the point: report it and offer to deploy that one target. The app itself does not need deploying to be tested. -- Use list_runs to find recent runs (optionally filtered by path, creator, label, or status), then get_job_logs with a returned id to inspect a specific run's logs — without starting a new test run. -- To see what a flow run actually did per step — statuses and results across the whole execution tree, subflow steps and loop iterations included — use get_flow_run_details with the run id (it also works while the flow is still running). Pass step to read one step's result in full (capped at 12k chars). Prefer it over get_job_logs when you need step results rather than logs. +- Use list_runs to find recent runs (optionally filtered by path, creator, label, or status), then get_run with a returned id to see what that run was called with, what it returned and what it logged — without starting a new test run. +- get_run also covers what a flow run did per step — statuses and results across the whole execution tree, subflow steps and loop iterations included — and works while the flow is still running. Pass step to read one step's result in full (capped at 12k chars). - Use open_page to show a workspace page with filters applied — Runs, Schedules, Variables, Resources, Assets, Audit logs, or Workspace settings on a specific tab (e.g. "open the failed runs of f/foo/bar", "open the schedule for X", "open the git sync settings"). Carry over every filter the user described — Runs takes the page's whole filter set (time window, path, user, folder, label, tag, worker, trigger kind, args/result, ...), so don't drop a criterion just because it wasn't in the request's main clause. Only the pages listed for this user in the tool are available; don't offer pages that aren't listed. Don't use it as a substitute for list_runs when you just need the data yourself. - Whenever you ask the user to perform a manual step in the UI — fill in a resource's credentials, set a secret variable's value, adjust a schedule or setting — call open_page in the same message, targeted at that item (pass open with its path to land in its edit drawer, or the page's filters otherwise). Never just describe where to click. - When the user is happy with the changes and wants to review or deploy them, use open_page with page "compare" — it opens the Compare & Deploy review page.${ @@ -1376,7 +1371,7 @@ ${pipelineBullet} - Building a data pipeline: call open_preview(kind="pipeline", path="") as the FIRST step, before creating any node — this opens the pipeline editor the user reviews in. path is the folder, not an item; an empty or not-yet-created folder is fine (create_folder first if needed, then open it). Opening it registers build_pipeline_node / edit_pipeline_node — use ONLY those to add or change pipeline nodes, never write_script for a pipeline node — they apply directly as unsaved drafts on the canvas (no separate accept/reject step) that the user reviews and deploys. Do not write pipeline scripts without first opening the editor. - When debugging a running raw app, call get_app_runtime_logs to read the live preview's browser console output. It needs the raw app preview open (open_preview kind="raw_app"). - To inspect what actually rendered in a running raw app (verify an edit landed on screen, diagnose a blank/empty or wrong view, answer "what's showing"), use search_dom (regex over the live HTML) and read_dom (a line-numbered window). Pass a \`selector\` to scope to an element — prefer the selector from a DOM element chip the user attached — or omit it for the whole page. When a chip lists an \`app_path\`, pass it too so the RIGHT app is read (several previews can be open; a query without \`app_path\` hits the visible one). The DOM is read live and is never in context; no match means the element isn't rendered. Both need the raw app preview open. -- get_app_runtime_logs only shows the app's browser console. For the server-side logs of a backend runnable the app invoked (a backend. call), call list_app_runs to get that run's job_id from the live preview, then get_job_logs with it. Use this when a backend call errors or returns something unexpected. +- get_app_runtime_logs only shows the app's browser console. For the server-side logs of a backend runnable the app invoked (a backend. call), call list_app_runs to get that run's job_id from the live preview, then get_run with it. Use this when a backend call errors or returns something unexpected. ${ isChromiumBrowser() ? `- When the user raises how a raw app looks (something is off, or they want the design or layout improved), call take_screenshot to see what they are looking at before changing anything. Reach for it when the request is about appearance, not to review your own edits, which you can read back from the code. It needs the raw app preview open (open_preview kind="raw_app").` @@ -1560,36 +1555,6 @@ function variableToItem(variable: ListableVariable): WorkspaceItem { } } -// Compact metadata for one run. The raw Job carries args/result/logs/raw_code -// which can be huge — list_runs returns only what's needed to identify a run. -function summarizeRun(job: Job): Record { - const base = { - id: job.id, - job_kind: job.job_kind, - path: job.script_path, - created_by: job.created_by, - created_at: job.created_at, - started_at: job.started_at, - schedule_path: job.schedule_path, - is_flow_step: job.is_flow_step, - tag: job.tag, - worker: job.worker - } - if ('success' in job) { - // CompletedJob - return { - ...base, - status: job.canceled ? 'canceled' : job.success ? 'success' : 'failure', - duration_ms: job.duration_ms - } - } - // QueuedJob (running or still waiting in the queue) - return { - ...base, - status: job.running ? 'running' : 'queued' - } -} - // ============= App helpers ============= type BackendRunnableInput = z.infer @@ -3757,7 +3722,7 @@ export const globalTools: Tool<{}>[] = [ def: createToolDef( listRunsSchema, 'list_runs', - "List recent runs (jobs), most recent first. Optionally filter by path, creator, label, or status. Returns compact metadata only — use get_job_logs with a returned id to read a run's logs." + 'List recent runs (jobs), most recent first. Optionally filter by path, creator, label, or status. Returns compact metadata only — use get_run with a returned id to see what a run was called with, returned and logged.' ), planModeSafe: true, showDetails: true, @@ -3784,56 +3749,24 @@ export const globalTools: Tool<{}>[] = [ }, { def: createToolDef( - getFlowRunDetailsSchema, - 'get_flow_run_details', - "Inspect a flow run's execution tree: per-step statuses and truncated results, including subflow steps, loop iterations, branches, and retries. Works on running flows too. Pass step to fetch one step's result in full (up to 12k chars)." + getRunSchema, + 'get_run', + "Inspect one run: its status, arguments, result and logs, plus — for a flow — the per-step execution tree with each step's status and truncated result (subflow steps, loop iterations, branches and retries included). Works on running jobs too. Pass step to fetch one step's result in full (up to 12k chars)." ), planModeSafe: true, showDetails: true, fn: async ({ args, workspace, toolId, toolCallbacks }) => { - const parsed = getFlowRunDetailsSchema.parse(args) + const parsed = getRunSchema.parse(args) toolCallbacks.setToolStatus(toolId, { content: parsed.step ? `Fetching result of step ${parsed.step} in run ${parsed.id}...` - : `Inspecting flow run ${parsed.id}...` + : `Inspecting run ${parsed.id}...` }) - const result = await getFlowRunDetails(workspace, parsed.id, parsed.step) + const result = await getRun(workspace, parsed.id, parsed.step) toolCallbacks.setToolStatus(toolId, { content: parsed.step ? `Fetched result of step ${parsed.step} in run ${parsed.id}` - : `Inspected flow run ${parsed.id}`, - result - }) - return result - } - }, - { - def: createToolDef( - getJobLogsSchema, - 'get_job_logs', - 'Fetch the logs of a job by its id. Use this to inspect the output of an existing run.' - ), - planModeSafe: true, - showDetails: true, - fn: async ({ args, workspace, toolId, toolCallbacks }) => { - const parsed = getJobLogsSchema.parse(args) - toolCallbacks.setToolStatus(toolId, { - content: `Fetching logs for job ${parsed.id}...` - }) - const logs = await JobService.getJobLogs({ - workspace, - id: parsed.id, - // Always suppress the "to remove ansi colors, use: sed ..." hint the - // backend otherwise prepends — it is noise for the model and is not - // actual ANSI stripping (the raw logs are returned either way). - removeAnsiWarnings: true - }) - const hasLogs = typeof logs === 'string' && logs.trim().length > 0 - const result = hasLogs ? logs : 'No logs available for this job.' - toolCallbacks.setToolStatus(toolId, { - content: hasLogs - ? `Fetched logs for job ${parsed.id}` - : `No logs available for job ${parsed.id}`, + : `Inspected run ${parsed.id}`, result }) return result @@ -6574,7 +6507,7 @@ async function testRunAppRunnable( toolId, startMessage: `Running backend runnable "${key}" of app "${path}"...`, // A path runnable pointing at a flow really does queue a flow job, so the - // failure path can offer get_flow_run_details; everything else is a script job. + // failure path can offer get_run's step tree; everything else is a script job. contextName: runnable.runType === 'flow' ? 'flow' : 'script', completionName: 'backend runnable', background: args.background, diff --git a/frontend/src/lib/components/copilot/chat/global/flowRunTree.test.ts b/frontend/src/lib/components/copilot/chat/global/flowRunTree.test.ts index 4b0dee5de0..7b2ddeb5fc 100644 --- a/frontend/src/lib/components/copilot/chat/global/flowRunTree.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/flowRunTree.test.ts @@ -190,4 +190,29 @@ describe('shapeFlowRunTree', () => { expect(parsed.steps[1].result).toBeUndefined() expect(parsed.steps[1].result_total_chars).toBe(700) }) + + it('spends the tree budget on the tree, not on the run payloads carried with it', () => { + const entries = [root()] + for (let i = 1; i <= 12; i++) { + entries.push( + entry({ + step_path: `s${i}`, + flow_step_id: `s${i}`, + status: 'failure', + success: false, + result_prefix: 'y'.repeat(700), + result_length: 700 + }) + ) + } + const logs = 'L'.repeat(12000) + const rendered = shapeFlowRunTree({ entries }, { status: 'failure', logs }) + + // Counting the overrides against the budget would shrink this tree away + // even though the tree itself fits: the model asked for the payloads. + expect(rendered.length).toBeGreaterThan(20000) + const parsed = JSON.parse(rendered) + expect(parsed.run.logs).toBe(logs) + expect(parsed.steps[0].result.length).toBe(700) + }) }) diff --git a/frontend/src/lib/components/copilot/chat/global/flowRunTree.ts b/frontend/src/lib/components/copilot/chat/global/flowRunTree.ts index 207dca1789..7f63de373b 100644 --- a/frontend/src/lib/components/copilot/chat/global/flowRunTree.ts +++ b/frontend/src/lib/components/copilot/chat/global/flowRunTree.ts @@ -1,19 +1,25 @@ -import { JobService, type GetFlowAllResultsResponse } from '$lib/gen' +import { JobService, type Job, type GetFlowAllResultsResponse } from '$lib/gen' +import { isWindmillTooBigObject } from '$lib/components/job_args' /** - * Model-facing view of a flow run's execution tree for the global chat's - * get_flow_run_details tool. The backend endpoint (get_flow_all_results) - * enumerates every job of the tree with per-entry truncated results; this - * module shapes that flat list into a compact per-step tree the model can - * read in one tool result. Step addresses ('b/c', 'b[12]/c') are resolved - * server-side by the same endpoint for full-result drill-down. + * Model-facing view of a run for the global chat's get_run tool: the job's own + * summary, args, result and logs, plus — when the run has steps — its execution + * tree. The backend endpoint (get_flow_all_results) enumerates every job of the + * tree with per-entry truncated results; this module shapes that flat list into + * a compact per-step tree the model can read in one tool result. Step addresses + * ('b/c', 'b[12]/c') are resolved server-side by the same endpoint for + * full-result drill-down. + * + * `summarizeRun` also backs list_runs' per-job summary, so it lives here rather + * than in core.ts: get_run needs it, and importing it back would be circular. */ export type FlowResultEntry = GetFlowAllResultsResponse['entries'][number] /** Per-entry result budget requested from the server for the tree view. */ export const TREE_RESULT_HEAD_CHARS = 700 -/** Cap on a drilled single-step full result handed to the model. */ +/** Cap on a full payload handed to the model: a drilled step result, and the + * run's own args, result and logs. */ export const STEP_RESULT_MAX_CHARS = 12000 /** Cap on the whole rendered tree; heads shrink progressively to fit. */ const TREE_TOTAL_BUDGET_CHARS = 20000 @@ -97,6 +103,13 @@ function sliceCodePointSafe(s: string, maxUnits: number): string { return last >= 0xd800 && last <= 0xdbff ? cut.slice(0, -1) : cut } +function sliceCodePointSafeEnd(s: string, maxUnits: number): string { + const cut = s.slice(-maxUnits) + const first = cut.charCodeAt(0) + // drop a leading lone low surrogate + return first >= 0xdc00 && first <= 0xdfff ? cut.slice(1) : cut +} + function shapeResult( entry: FlowResultEntry, opts: ShapeOpts @@ -211,7 +224,8 @@ function shapeChildren(children: FlowTreeNode[], opts: ShapeOpts): Record ): Record { const run = shapeStep(root, opts) const steps = run.steps @@ -221,19 +235,41 @@ function renderTree( if (!steps && root.entry.kind !== 'flow' && root.entry.kind !== 'flowpreview') { run.label = `Job (${root.entry.kind})` } + if (runOverrides) { + // A replacement result supersedes the tree entry's head, and the stale + // truncation marker must go with it. Without one the head stays: it is a + // real server-side prefix of a payload `getJob` would only hand back as a + // WINDMILL_TOO_BIG placeholder. + if ('result' in runOverrides) { + delete run.result + delete run.result_total_chars + } + Object.assign(run, runOverrides) + } return { ...(rootJobNote ? { note: rootJobNote } : {}), run, - ...(steps ? { steps } : {}), - hint: `Results are truncated. Call get_flow_run_details again with step="" (e.g. "b/c", or "b[12]" for one loop iteration) for a step's result in full (up to ${STEP_RESULT_MAX_CHARS} chars).` + ...(steps + ? { + steps, + hint: `Step results are truncated. Call get_run again with step="" (e.g. "b/c", or "b[12]" for one loop iteration) for a step's result in full (up to ${STEP_RESULT_MAX_CHARS} chars), or with id set to a step's job_id for that step's own args, result and logs.` + } + : {}) } } /** Render the whole tree, shrinking result heads until it fits the budget. */ -export function shapeFlowRunTree(response: GetFlowAllResultsResponse): string { +export function shapeFlowRunTree( + response: GetFlowAllResultsResponse, + runOverrides?: Record +): string { const root = buildFlowTree(response.entries) if (!root) { - return 'No jobs found for this run.' + // The tree is the optional half: what the caller already read of the job + // still answers the question, so don't drop it with the missing tree. + return runOverrides + ? JSON.stringify({ run: runOverrides }, null, 1) + : 'No jobs found for this run.' } const notes = [ ...(response.enclosing_job @@ -252,34 +288,188 @@ export function shapeFlowRunTree(response: GetFlowAllResultsResponse): string { ] const rootJobNote = notes.length > 0 ? notes.join(' ') : undefined + // The budget caps the tree, not the run's own args/result/logs — those are + // capped on their own and the model asked for them, so they don't count here + // and the last-resort slice keeps room for them (`run` precedes `steps`). + const overrideChars = runOverrides ? JSON.stringify(runOverrides, null, 1).length : 0 let rendered = '' for (const opts of SHRINK_LADDER) { - rendered = JSON.stringify(renderTree(root, rootJobNote, opts), null, 1) - if (rendered.length <= TREE_TOTAL_BUDGET_CHARS) { + rendered = JSON.stringify(renderTree(root, rootJobNote, opts, runOverrides), null, 1) + if (rendered.length - overrideChars <= TREE_TOTAL_BUDGET_CHARS) { return rendered } } return ( - rendered.slice(0, TREE_TOTAL_BUDGET_CHARS) + + rendered.slice(0, TREE_TOTAL_BUDGET_CHARS + overrideChars) + `\n… (tree truncated at ${TREE_TOTAL_BUDGET_CHARS} chars — drill into specific steps with the step parameter)` ) } -/** Entry point of the get_flow_run_details tool. Without `step`: the compact - * tree. With `step`: that job's full (capped) result, resolved server-side. */ -export async function getFlowRunDetails( - workspace: string, - id: string, - step?: string -): Promise { +// Compact metadata for one run. The raw Job carries args/result/logs/raw_code +// which can be huge — this is only what's needed to identify a run, so list_runs +// can return one entry per job. +export function summarizeRun(job: Job): Record { + const base = { + id: job.id, + job_kind: job.job_kind, + path: job.script_path, + created_by: job.created_by, + created_at: job.created_at, + started_at: job.started_at, + schedule_path: job.schedule_path, + is_flow_step: job.is_flow_step, + tag: job.tag, + worker: job.worker + } + if ('success' in job) { + // CompletedJob. `success` is true for a skipped job too, so is_skipped has + // to be read first or a skipped step reports as a successful one. + return { + ...base, + status: job.canceled + ? 'canceled' + : job.is_skipped + ? 'skipped' + : job.success + ? 'success' + : 'failure', + duration_ms: job.duration_ms + } + } + // QueuedJob. A running job with suspends outstanding is parked — on an approval + // step or on a parallelism slot — not working, and `running` alone hides that. + return { + ...base, + status: job.running ? (job.suspend ? 'suspended' : 'running') : 'queued' + } +} + +/** Cap a payload to STEP_RESULT_MAX_CHARS. `tail` keeps the end instead of the + * start — for logs, where the failure is at the bottom. + * + * Counts and cuts without materialising the string: logs arrive whole and + * unbounded, and spreading one into an array of code points costs ~9x its size + * (a 10MB log measured +90MB). Cutting in UTF-16 units keeps at most the budget + * in code points, never more, so an astral-heavy payload is trimmed slightly + * shorter than advertised rather than overshooting. */ +function cap(text: string, tail = false): string { + // UTF-16 length is never below the code-point count, so anything passing this + // is already under the cap and needs no counting pass at all. + if (text.length <= STEP_RESULT_MAX_CHARS) return text + const total = countCodePoints(text) + if (total <= STEP_RESULT_MAX_CHARS) return text + const note = `… (truncated: ${total} chars total)` + return tail + ? `${note}\n${sliceCodePointSafeEnd(text, STEP_RESULT_MAX_CHARS)}` + : `${sliceCodePointSafe(text, STEP_RESULT_MAX_CHARS)}\n${note}` +} + +/** Told apart from an empty log so the model doesn't report "no logs" for a run + * whose logs it simply failed to read. */ +const LOGS_UNREADABLE = 'Logs could not be read for this run.' + +/** `getJob` swaps a payload over ~90KB for a marker rather than sending it + * (`get_job_query!` in backend/windmill-api/src/jobs.rs), and the two fields use + * different ones: a result becomes the bare string, args become exactly + * `{reason: }`. Each field matches only its own form, so a payload that + * merely carries that string in a `reason` of its own stays the run's value. */ +const TOO_BIG_RESULT = 'WINDMILL_TOO_BIG' + +function stringify(value: unknown): string { + return typeof value === 'string' ? value : JSON.stringify(value, null, 1) +} + +/** An elided args/result payload is reported, never fetched around: the + * endpoints that return those whole take no length parameter, so recovering a + * usable head would mean pulling the entire payload (up to MAX_RESULT_SIZE_MB, + * 500 by default) into the tab to keep 12k of it. The flag is the whole report — + * the result keeps the head the step tree already carries beside it. */ +function shapeRunArgs(job: Job): Record { + if (!job.args) return {} + return isWindmillTooBigObject(job.args) + ? { args_truncated: true } + : { args: cap(stringify(job.args)) } +} + +function shapeRunResult(job: Job): Record { + if (!('result' in job) || job.result === undefined) return {} + // No `result` key when elided, so the tree's `result_prefix` — a real + // server-side head of the same payload — stands in its place unoverridden. + return job.result === TOO_BIG_RESULT + ? { result_truncated: true } + : { result: cap(stringify(job.result)) } +} + +/** Why a run ended badly. get_run is the only route to these — the catalog + * refuses getJob. Kept out of summarizeRun, which list_runs pays per job. */ +function diagnoseRun(job: Job): Record { + return { + ...(job.canceled_by ? { canceled_by: job.canceled_by } : {}), + ...(job.canceled_reason ? { canceled_reason: job.canceled_reason } : {}), + ...(job.mem_peak ? { mem_peak_kb: job.mem_peak } : {}) + } +} + +/** Entry point of the get_run tool. Without `step`: the run's summary, args, + * result and logs, plus the per-step tree when the run has steps. With `step`: + * that step's full (capped) result, resolved server-side. */ +export async function getRun(workspace: string, id: string, step?: string): Promise { if (!step) { - return shapeFlowRunTree( - await JobService.getFlowAllResults({ workspace, id, maxResultLen: TREE_RESULT_HEAD_CHARS }) - ) + // Only the job read is load-bearing: logs and the step tree each answer + // part of the question, so neither failing should cost the model the rest. + const [job, logs, results] = await Promise.all([ + JobService.getJob({ workspace, id, noLogs: true, noCode: true }), + // The dedicated endpoint rather than the job's own `logs` field: that one + // is the last 20k still in the DB column, missing the head that log + // compaction flushed to object storage. This one stitches them back. + // + // It takes no length parameter, so unlike args and result the whole log + // does come into the tab before being capped. Only this job's own logs, + // though: a flow's are its orchestration lines, not its steps'. + JobService.getJobLogs({ + workspace, + id, + // Suppress the "to remove ansi colors, use: sed ..." hint the backend + // otherwise prepends — noise for the model, and not actual stripping. + removeAnsiWarnings: true + }).catch(() => LOGS_UNREADABLE), + // `.catch` alone would leave the tree half-guarded: like the log read, this + // one can fail by resolving `undefined` rather than rejecting, and reading + // `.entries` off that throws — losing the job and logs already in hand. + JobService.getFlowAllResults({ workspace, id, maxResultLen: TREE_RESULT_HEAD_CHARS }) + .catch(() => undefined) + .then((r) => r ?? ({ entries: [] } as GetFlowAllResultsResponse)) + ]) + const { id: _id, ...summary } = summarizeRun(job) + const payloads = { ...shapeRunArgs(job), ...shapeRunResult(job) } + // A read that fails without rejecting still arrives here: the generated client + // resolves `undefined` when it cannot read the body (a truncated response, a + // dropped connection). Logs are the one payload where empty is a real answer, + // so that has to be told apart from an empty log rather than reported as one. + const shapedLogs = + typeof logs !== 'string' || logs === LOGS_UNREADABLE + ? LOGS_UNREADABLE + : logs.trim() + ? cap(logs, true) + : 'No logs for this run.' + return shapeFlowRunTree(results, { + ...summary, + ...diagnoseRun(job), + // A successful read always carries the job itself as the root entry, so + // no entries means the read failed — and nothing else would name the run. + ...(results.entries.length === 0 ? { job_id: id, steps_unavailable: true } : {}), + ...payloads, + logs: shapedLogs + }) } - // Drill-down: the server resolves the address directly (a few indexed - // lookups, no tree enumeration) and returns the single job as an entry. + return getStepResult(workspace, id, step) +} + +/** One step's result in full, addressed by step path. The server resolves the + * address directly (a few indexed lookups, no tree enumeration) and returns the + * single job as an entry. */ +async function getStepResult(workspace: string, id: string, step: string): Promise { const response = await JobService.getFlowAllResults({ workspace, id, diff --git a/frontend/src/lib/components/copilot/chat/shared.ts b/frontend/src/lib/components/copilot/chat/shared.ts index 73efd2e37e..13799cf09f 100644 --- a/frontend/src/lib/components/copilot/chat/shared.ts +++ b/frontend/src/lib/components/copilot/chat/shared.ts @@ -671,6 +671,14 @@ export type AssistantDisplayMessage = BaseDisplayMessage & { * would look like it is still streaming forever. */ streaming?: boolean + /** Flow step that produced this message, when the conversation is a flow run + * rather than a copilot turn. Rendered as a label above the content. */ + stepName?: string + /** The run behind this answer. Flow chats only: a copilot turn happens in the + * browser and has no job. */ + jobId?: string + /** When the message was stored, as the server reports it. */ + createdAt?: string } /** @@ -1853,13 +1861,13 @@ export async function buildTestRunArgs( } // The string handed back to the model when a job is backgrounded. It carries the -// job id so the model can pull status/logs on demand (get_job_logs / list_runs), +// job id so the model can pull status/args/result/logs on demand (get_run / list_runs), // and tells it the completion will be reported later (notify-only wake). function backgroundedSummary(jobId: string, label: string): string { return ( `Job ${jobId} for "${label}" is taking a while and is now running in the background — ` + `the chat is free to continue and you'll be told when it finishes. ` + - `To inspect it now, call get_job_logs with id="${jobId}" (or list_runs); ` + + `To inspect it now, call get_run with id="${jobId}" (or list_runs); ` + `to stop it, call cancel_job with id="${jobId}".` ) } @@ -1890,7 +1898,7 @@ export function completedJobToolStatus(job: CompletedJob): Partial { }) const summary = formatResultSummary(job.result, job.logs, job.success) - // get_flow_run_details only exists in the global/sessions chat (the same - // hosts that wire the job hooks) — don't advertise it to in-editor chats. + // get_run only exists in the global/sessions chat (the same hosts that wire + // the job hooks) — don't advertise it to in-editor chats. if (detachEnabled && config.contextName === 'flow' && !job.success) { return ( summary + - `\n\nFor per-step statuses and results (subflow steps included), call get_flow_run_details with id="${jobId}".` + `\n\nFor per-step statuses and results (subflow steps included), call get_run with id="${jobId}".` ) } return summary diff --git a/frontend/src/lib/components/copilot/chatModelSettings.test.ts b/frontend/src/lib/components/copilot/chatModelSettings.test.ts new file mode 100644 index 0000000000..04d1834556 --- /dev/null +++ b/frontend/src/lib/components/copilot/chatModelSettings.test.ts @@ -0,0 +1,246 @@ +import { describe, expect, it } from 'vitest' +import { + carriedReasoning, + fixedReasoningReason, + reasoningControlState, + reasoningDisplay, + REASONING_PROVIDER_DEFAULT, + type ChatModelSettingsReasoning +} from './chatModelSettings' +import { + getReasoningCapability, + REASONING_OFF, + resolveEffectiveReasoning +} from './reasoningRegistry' + +/** + * The trigger's suffix and the slider's stops are read side by side, so they have to agree + * about one value — the provider-native off token must not read as `none` on one and `off` + * on the other, and an effort the run does not send must not be named at all. + */ +function display( + reasoning: Partial & { provider: any; model: string } +) { + const full = { + value: undefined, + offToken: undefined, + sendsDefaultWhenUnset: false, + onSelect: () => {}, + ...reasoning + } as ChatModelSettingsReasoning + const capability = getReasoningCapability(full.provider, full.model) + // Composed exactly as the component composes it, so the test exercises the real pair. + const effective = resolveEffectiveReasoning({ + provider: full.provider, + model: full.model, + reasoning: full.value + }) + return reasoningDisplay(full, capability, effective) +} + +describe('reasoningDisplay', () => { + it('says nothing for a model that cannot reason', () => { + const shown = display({ provider: 'openai', model: 'gpt-4o' }) + expect(shown.label).toBeUndefined() + expect(shown.stops).toEqual([]) + }) + + // The session chat's own sentinel: what it stores is already the word the reader sees. + it('reads the session chat off sentinel as off', () => { + const shown = display({ + provider: 'openai', + model: 'gpt-5.1', + offToken: REASONING_OFF, + value: REASONING_OFF, + sendsDefaultWhenUnset: true + }) + expect(shown.label).toBe(REASONING_OFF) + expect(shown.currentStop).toBe(REASONING_OFF) + }) + + // An agent writes the provider's own token, which can read as anything. + it('reads a provider-native off token as off too', () => { + const shown = display({ + provider: 'openai', + model: 'gpt-5.1', + offToken: 'none', + value: 'none' + }) + expect(shown.label).toBe(REASONING_OFF) + expect(shown.currentStop).toBe('none') + expect(shown.stops[0]).toBe('none') + }) + + it('names the level a chat that fills one in will send', () => { + const shown = display({ + provider: 'openai', + model: 'gpt-5.1', + offToken: REASONING_OFF, + value: undefined, + sendsDefaultWhenUnset: true + }) + expect(shown.label).toBe('high') + }) + + // An agent step omits the field, so naming a level would claim something untrue. + it('names no level where an unset effort is simply not sent', () => { + const shown = display({ + provider: 'anthropic', + model: 'claude-sonnet-5', + offToken: 'none', + value: undefined + }) + expect(shown.label).toBe(REASONING_PROVIDER_DEFAULT) + expect(shown.currentStop).toBe('') + }) + + // Claude 4.x only thinks when asked, so an absent effort is already off — and the flow + // chat must be able to get back to it after a level has been picked. + it('offers omission as the off stop where that is how the model disables', () => { + const unset = display({ provider: 'anthropic', model: 'claude-opus-4-6', offToken: '' }) + expect(unset.label).toBe(REASONING_OFF) + expect(unset.stops[0]).toBe('') + const picked = display({ + provider: 'anthropic', + model: 'claude-opus-4-6', + offToken: '', + value: 'high' + }) + expect(picked.currentStop).toBe('high') + expect(picked.stops).toContain('') + }) + + // gpt-5 reasons at medium with no effort sent, so an empty off token buys no off stop. + it('offers no off where the model cannot stop thinking', () => { + const shown = display({ provider: 'openai', model: 'gpt-5', offToken: '' }) + expect(shown.stops).not.toContain('') + expect(shown.label).toBe(REASONING_PROVIDER_DEFAULT) + }) + + // The run sends an explicitly set effort whatever the model, so a token typed against a + // provider we have no rules for has to reach the trigger — silence would hide it. + it('names a set effort even where it can offer no ladder', () => { + const shown = display({ provider: 'customai', model: 'deepseek-r1', value: 'high' }) + expect(shown.label).toBe('high') + expect(shown.stops).toEqual([]) + }) +}) + +describe('carriedReasoning', () => { + const cap = (model: string) => getReasoningCapability('openai', model) + + // A level carried onto a model that cannot think stays in the flow input, and the run sends it. + it('drops a level the new model does not have', () => { + expect(carriedReasoning('high', '', cap('gpt-4o'))).toBeUndefined() + expect(carriedReasoning('xhigh', '', cap('gpt-5.1'))).toBeUndefined() + }) + + it('keeps a level the new model does have', () => { + expect(carriedReasoning('high', '', cap('gpt-5.1'))).toBe('high') + }) + + it('carries off only onto a model that can truly stop thinking', () => { + expect(carriedReasoning(REASONING_OFF, REASONING_OFF, cap('gpt-5.1'))).toBe(REASONING_OFF) + expect(carriedReasoning(REASONING_OFF, REASONING_OFF, cap('gpt-5'))).toBeUndefined() + }) + + // A provider the registry has no rules for draws no thinking control, so a carried level + // would be invisible and unclearable — and still sent, since an explicitly set effort + // goes out whatever the model. + it('drops the effort where it has no rules for the provider', () => { + expect( + carriedReasoning('high', REASONING_OFF, getReasoningCapability('customai', 'deepseek-r1')) + ).toBeUndefined() + }) + + it('has nothing to carry when no effort is set', () => { + expect(carriedReasoning(undefined, '', cap('gpt-5.1'))).toBeUndefined() + expect(carriedReasoning('', '', cap('gpt-5.1'))).toBeUndefined() + }) +}) + +const asReasoning = (over: Partial): ChatModelSettingsReasoning => + ({ + provider: 'openai', + model: 'gpt-5.1', + value: undefined, + offToken: REASONING_OFF, + sendsDefaultWhenUnset: false, + writable: true, + typedWhenUnknown: true, + onSelect: () => {}, + ...over + }) as ChatModelSettingsReasoning + +/** The control is always drawn; this is the only thing that decides what it draws. */ +describe('reasoningControlState', () => { + const cap = (model: string) => getReasoningCapability('openai', model) + + it('shows the ladder for a model with levels', () => { + expect(reasoningControlState(asReasoning({}), cap('gpt-5.1'))).toBe('ladder') + }) + + it('says a model cannot think when the registry knows it cannot', () => { + expect(reasoningControlState(asReasoning({ model: 'gpt-4o' }), cap('gpt-4o'))).toBe( + 'unsupported' + ) + }) + + // Not the same as "cannot think": we have no rules for the provider, so the flow's own + // token is typed rather than picked. + it('asks for a typed token where it has no rules for the provider', () => { + expect( + reasoningControlState( + asReasoning({ provider: 'customai', model: 'deepseek-r1' }), + getReasoningCapability('customai', 'deepseek-r1') + ) + ).toBe('unknown') + }) + + // The session chat has no typed effort: a provider with no rules reads as unable to think. + it('offers no typed token to a chat that does not take one', () => { + expect( + reasoningControlState( + asReasoning({ provider: 'customai', model: 'deepseek-r1', typedWhenUnknown: false }), + getReasoningCapability('customai', 'deepseek-r1') + ) + ).toBe('unsupported') + }) + + // A provider with a full ladder must not be described as unreadable just because no + // model has been picked yet — which is the state right after choosing a resource. + it('waits for a model rather than blaming the provider', () => { + expect( + reasoningControlState(asReasoning({ model: undefined }), { supported: false, known: false }) + ).toBe('awaiting-model') + }) + + it('shows what the flow fixed when this chat cannot write it', () => { + expect(reasoningControlState(asReasoning({ writable: false }), cap('gpt-5.1'))).toBe('fixed') + }) +}) + +describe('fixedReasoningReason', () => { + it('names the level the run will use', () => { + expect( + fixedReasoningReason(asReasoning({ value: 'high' }), { supported: true, known: true }) + ).toBe('high · set in the flow') + }) + + // The step naming no effort at all is the common shape; saying it was "set in the flow" + // would describe a line the flow does not contain. + it('does not claim a level the step never set', () => { + expect( + fixedReasoningReason(asReasoning({ value: undefined }), { supported: true, known: true }) + ).toBe('Not set in the flow, so the provider decides') + }) + + it('surfaces a level fixed on a model that cannot use it', () => { + expect( + fixedReasoningReason(asReasoning({ value: 'high', model: 'gpt-4o' }), { + supported: false, + known: true + }) + ).toContain('cannot think') + }) +}) diff --git a/frontend/src/lib/components/copilot/chatModelSettings.ts b/frontend/src/lib/components/copilot/chatModelSettings.ts new file mode 100644 index 0000000000..3fce0270d1 --- /dev/null +++ b/frontend/src/lib/components/copilot/chatModelSettings.ts @@ -0,0 +1,219 @@ +import type { AIProvider } from '$lib/gen' +import type { Item } from '$lib/utils' +import { REASONING_OFF } from './reasoningRegistry' + +/** + * The contract between a chat and its model button. + * + * One component renders this menu for every chat — the copilot's own session chat and + * the flow chat — so the component knows only about rows, choices and a reasoning + * ladder. What a row means (a workspace AI resource, a prompt to edit, a reading + * preference) is the caller's business, and each caller derives its own config: a fixed + * one for the session chat, one derived from the flow's exposed inputs for flow chat. + */ + +export type ModelChoice = { + /** Stable across rebuilds of the config; used as the `{#each}` key. */ + key: string + label: string + /** Muted trailing text, e.g. the provider a resource speaks. */ + hint?: string + selected: boolean + onSelect: () => void +} + +export type ChoiceSection = { + /** Section heading, e.g. 'Provider' or 'Model'. */ + label: string + options: ModelChoice[] + /** Fetched lists render one consistent loading line instead of the options. */ + loading?: boolean + /** Shown when the list is empty and settled. */ + emptyMessage?: string + maxHeight?: string + /** + * A typed entry under the options, for a value the list does not hold: an endpoint with no + * listing, or a model newer than the catalogue. An empty entry commits nothing. + */ + custom?: { placeholder: string; onCommit: (value: string) => void } +} + +export type ChatModelSettingsConfig = { + /** The trigger's main text: the chosen model, or an invitation to choose one. */ + label: string + title?: string + /** Trailing pill on the trigger, e.g. the free-tier grant this chat is spending. */ + badge?: { text: string; warn?: boolean } + /** Nothing here is editable — the trigger still names the model, but no menu opens. */ + readOnly?: boolean + readOnlyReason?: string + /** + * Rows above and below the choice sections. Given the menu's own `close` because a + * row that opens a modal must close the menu first, while a row that toggles a + * preference must not. + */ + topItems?: (close: () => void) => Item[] + sections?: ChoiceSection[] + bottomItems?: (close: () => void) => Item[] + /** + * The thinking slider. The ladder is derived from the provider and model here rather + * than by each caller, so a new provider's effort levels reach every chat at once. + * `value` is the raw stored effort (undefined meaning the model's default), and + * `offToken` the token this caller stores for "off" — the copilot keeps its own + * sentinel and translates when it calls the provider, an agent writes the + * provider-native token straight into its step. + */ + reasoning?: ChatModelSettingsReasoning +} + +export type ChatModelSettingsReasoning = { + /** Absent until the chat knows what it will run; the ladder then has nothing to stand on. */ + provider: AIProvider | undefined + model: string | undefined + value: string | undefined + offToken: string | undefined + /** + * What an unset value means on the wire. The copilot fills one in before calling the + * provider, so unset really runs at the default effort and the button says so. An agent + * step omits the field entirely, so unset means whatever the provider does by itself — + * naming a level there would state something the run does not do. + */ + sendsDefaultWhenUnset: boolean + /** + * Whether this chat can write the effort back. False where the flow fixes it in the step: + * the run still uses it, so the button shows it and refuses to pretend otherwise — only + * the flow editor can change it. + */ + writable: boolean + /** + * Whether a provider the registry has no rules for gets a typed effort field. True for an + * agent, which writes the token straight into its step. False for the copilot, which then + * shows the model as unable to think. + */ + typedWhenUnknown: boolean + onSelect: (token: string) => void +} + +/** What an unset agent effort reads as: the provider decides, and we do not know what. */ +export const REASONING_PROVIDER_DEFAULT = 'default' + +/** + * The effort to keep when the model changes, or nothing where the new model has no such + * level. Dropped rather than carried because a model that cannot think at that level either + * rejects the request or quietly runs at another one, and the button would name a level the + * run never used. Off survives only onto a model that can truly disable. + * + * A model the registry has no rules for drops it too, for the same reason: a chat without + * `typedWhenUnknown` draws no thinking control there, so a carried level would be invisible + * and unclearable while still going out on the wire — `resolveEffectiveReasoning` sends an explicitly set effort + * whatever the model, and a provider that rejects the field would then fail every turn with + * nothing on screen to explain it. + */ +export function carriedReasoning( + current: string | undefined, + offToken: string | undefined, + capability: { levels: string[]; canDisable: boolean } +): string | undefined { + if (current === undefined || current === '') return undefined + if (offToken !== undefined && current === offToken) { + return capability.canDisable ? current : undefined + } + return capability.levels.includes(current) ? current : undefined +} + +/** + * What the menu shows for the reasoning ladder: the stops the slider offers, the one it + * sits on, and the suffix on the trigger. + * + * Pure and here rather than in the component because these three have to agree — a stop + * the slider renders as `off` must not read as the provider's own `none` on the button — + * and because the rules are provider-shaped enough to be worth testing directly. + */ +export function reasoningDisplay( + reasoning: ChatModelSettingsReasoning | undefined, + capability: { supported: boolean; levels: string[]; canDisable: boolean }, + effective: string | undefined +): { + stops: string[] + currentStop: string + /** Trigger suffix, or undefined when there is nothing truthful to say. */ + label: string | undefined +} { + if (!reasoning) return { stops: [], currentStop: '', label: undefined } + if (!capability.supported) { + // No ladder to place it on, but an effort that is explicitly set still goes out — + // `resolveEffectiveReasoning` sends one whatever the model — so the button names it. + // Saying nothing would hide from the reader what the run is about to do. + const set = reasoning.value ? reasoning.value : undefined + return { stops: [], currentStop: '', label: set } + } + // An off position only where the model can truly disable, else the provider would + // coerce it to the lowest level; then the provider-native levels. + const offToken = capability.canDisable ? reasoning.offToken : undefined + const stops = [...(offToken !== undefined ? [offToken] : []), ...capability.levels] + // An agent whose model disables by omission stores the empty string, which the run + // treats as no effort at all — so an unset value already sits on that stop. + const isOff = offToken !== undefined && (reasoning.value ?? '') === offToken + if (isOff) { + // The off token is provider-native and can read as anything ('none', 'disabled'); + // on the button and on the slider it always reads as off. + return { stops, currentStop: offToken as string, label: REASONING_OFF } + } + if (reasoning.value === undefined || reasoning.value === '') { + // Where the provider takes an explicit disable, unset is a third state — the + // provider's own level, above off — that the ladder has no position for. The + // button still names it, and every stop the ladder does offer stays reachable. + return reasoning.sendsDefaultWhenUnset + ? { stops, currentStop: effective ?? '', label: effective ?? REASONING_OFF } + : { stops, currentStop: '', label: REASONING_PROVIDER_DEFAULT } + } + return { stops, currentStop: reasoning.value, label: reasoning.value } +} + +/** Which thinking control a chat should draw. */ +export type ReasoningControlState = + /** The flow sets the effort itself; show what the run will use. */ + | 'fixed' + /** No model chosen yet, so nothing can be said about its levels. */ + | 'awaiting-model' + /** No rules for this provider, and the chat takes a typed token. */ + | 'unknown' + /** Known levels — the ladder. */ + | 'ladder' + /** Known to have none. */ + | 'unsupported' + +/** + * The control is always drawn; only its state varies. Decided here rather than in the markup + * so the states sit in one readable, testable place. + */ +export function reasoningControlState( + reasoning: ChatModelSettingsReasoning | undefined, + capability: { supported: boolean; known: boolean } +): ReasoningControlState { + if (!reasoning || !reasoning.writable) return 'fixed' + if (!reasoning.model) return 'awaiting-model' + if (!capability.known && reasoning.typedWhenUnknown) return 'unknown' + return capability.supported ? 'ladder' : 'unsupported' +} + +/** + * What the row says when the chat cannot write the effort. Naming the level is the point: a + * button that names the model a run will use should name its thinking too, and a level fixed + * on a model that cannot use one is a broken flow worth seeing rather than a silent row. + */ +export function fixedReasoningReason( + reasoning: ChatModelSettingsReasoning | undefined, + capability: { supported: boolean; known: boolean } +): string { + const cannotThink = capability.known && !capability.supported + const model = reasoning?.model ?? 'this model' + if (!reasoning?.value) { + // The step names no effort, so the provider decides — saying it was "set in the flow" + // would describe a line the flow does not contain. + return cannotThink ? `${model} cannot think` : 'Not set in the flow, so the provider decides' + } + return cannotThink + ? `${reasoning.value} · set in the flow, but ${model} cannot think` + : `${reasoning.value} · set in the flow` +} diff --git a/frontend/src/lib/components/copilot/reasoningRegistry.test.ts b/frontend/src/lib/components/copilot/reasoningRegistry.test.ts index b4c935bcaa..faf6f60264 100644 --- a/frontend/src/lib/components/copilot/reasoningRegistry.test.ts +++ b/frontend/src/lib/components/copilot/reasoningRegistry.test.ts @@ -77,9 +77,9 @@ describe('supportsReasoning (static registry)', () => { } // Bedrock translates the same sentinel on its Converse path, but only for // Opus 5 — AWS documents Bedrock's Sonnet 5 as always thinking. - expect( - getReasoningCapability('aws_bedrock', 'global.anthropic.claude-opus-5').canDisable - ).toBe(true) + expect(getReasoningCapability('aws_bedrock', 'global.anthropic.claude-opus-5').canDisable).toBe( + true + ) expect( resolveRequestReasoning({ provider: 'aws_bedrock', @@ -189,13 +189,29 @@ describe('supportsReasoning (static registry)', () => { expect(supportsReasoning('mistral', 'mistral-medium-3.5')).toBe(true) expect(getReasoningCapability('mistral', 'mistral-medium-3-5').canDisable).toBe(true) }) - it('returns no levels for providers without a registry entry', () => { + it('returns no levels for a model its provider family has no entry for', () => { + // The family is known, so the `false` is an answer: codestral does not reason. expect(getReasoningCapability('mistral', 'codestral-latest')).toEqual({ supported: false, levels: [], - canDisable: false + canDisable: false, + known: true }) }) + + // `customai` fronts any OpenAI-compatible endpoint, so `supported: false` there is an + // absence of rules rather than a fact about the model. A caller that shows the reader + // "this model cannot think" has to tell the two apart. + it('admits when it has no rules for the provider at all', () => { + expect(getReasoningCapability('customai', 'deepseek-r1')).toEqual({ + supported: false, + levels: [], + canDisable: false, + known: false + }) + expect(getReasoningCapability('openai', 'gpt-4o').known).toBe(true) + expect(getReasoningCapability('anthropic', 'claude-sonnet-5').known).toBe(true) + }) it('only offers off where the model can truly disable thinking', () => { // Gemini Pro enforces a thinking floor — no off option. expect(getReasoningCapability('googleai', 'gemini-2.5-pro').canDisable).toBe(false) diff --git a/frontend/src/lib/components/copilot/reasoningRegistry.ts b/frontend/src/lib/components/copilot/reasoningRegistry.ts index 95a00e28c8..242f6cb329 100644 --- a/frontend/src/lib/components/copilot/reasoningRegistry.ts +++ b/frontend/src/lib/components/copilot/reasoningRegistry.ts @@ -231,14 +231,35 @@ export type ReasoningCapability = { * level, making the switch a lie. */ canDisable: boolean + /** + * Whether `supported` is an answer or an absence of one. The registry has rules per + * provider family and falls through to `false` for the rest — `customai` above all, + * which fronts any OpenAI-compatible endpoint and may well serve a thinking model. A + * caller that presents `supported: false` as a fact must check this first, or it tells + * the reader a model cannot think when all we know is that we have never heard of it. + */ + known: boolean } +/** Provider families the registry has real rules for; everything else is a shrug. */ +const KNOWN_REASONING_FAMILIES: ReadonlySet = new Set([ + 'anthropic', + 'aws_bedrock', + 'openai', + 'azure_openai', + 'openrouter', + 'googleai', + 'deepseek', + 'mistral' +]) + /** Resolve the reasoning capability of a model from the static registry. */ export function getReasoningCapability(provider: AIProvider, model: string): ReasoningCapability { const bareModel = stripLegacyThinkingSuffix(model) + const known = KNOWN_REASONING_FAMILIES.has(reasoningProviderFamily(provider, bareModel)) const supported = supportsReasoningStatic(provider, bareModel) if (!supported) { - return { supported: false, levels: [], canDisable: false } + return { supported: false, levels: [], canDisable: false, known } } const family = reasoningProviderFamily(provider, bareModel) const levels = @@ -251,7 +272,7 @@ export function getReasoningCapability(provider: AIProvider, model: string): Rea : family === 'openrouter' ? openrouterReasoningLevels(bareModel) : (PROVIDER_REASONING_LEVELS[family] ?? ['low', 'medium', 'high']) - return { supported, levels, canDisable: canDisableReasoning(provider, bareModel) } + return { supported, levels, canDisable: canDisableReasoning(provider, bareModel), known } } /** @@ -362,15 +383,11 @@ export function explicitOffToken(provider: AIProvider, model: string): Reasoning // real off there and stays the wire form. Only the 5 family, which // thinks when the field is absent, needs the explicit disable — // Fable and Mythos reject it outright and get no off token at all. - return /claude-(opus|sonnet)-5/.test(model.toLowerCase()) - ? ANTHROPIC_OFF_SENTINEL - : undefined + return /claude-(opus|sonnet)-5/.test(model.toLowerCase()) ? ANTHROPIC_OFF_SENTINEL : undefined case 'aws_bedrock': // Bedrock's Sonnet 5 cannot be disabled at all, so only Opus 5 gets // the sentinel; the rest keep omission. - return model.toLowerCase().includes('claude-opus-5') - ? ANTHROPIC_OFF_SENTINEL - : undefined + return model.toLowerCase().includes('claude-opus-5') ? ANTHROPIC_OFF_SENTINEL : undefined case 'googleai': // Gemini 2.5/3 think by default (dynamic budget / level). The backend // proxy maps 'none' to off on Flash, or the floor on Pro (only diff --git a/frontend/src/lib/components/datatableUsableRoles.ts b/frontend/src/lib/components/datatableUsableRoles.ts index 0f53a540af..84e52f29a8 100644 --- a/frontend/src/lib/components/datatableUsableRoles.ts +++ b/frontend/src/lib/components/datatableUsableRoles.ts @@ -1,4 +1,5 @@ -import { WorkspaceService, type ListUsableDatatableRolesResponse } from '$lib/gen' +import { OpenAPI, type ListUsableDatatableRolesResponse } from '$lib/gen' +import { request } from '$lib/gen/core/request' import { isCloudHosted } from '$lib/cloud' import { ADMIN_DATATABLE_ROLE } from './dbTypes' @@ -24,7 +25,16 @@ export async function listUsableDatatableRoles( ): Promise { if (isCloudHosted()) return NOT_UNDER_ROLES try { - return await WorkspaceService.listUsableDatatableRoles({ workspace, datatableName }) + // The generated client encodes path params with `encodeURI`, which leaves a '?' in a + // data table name created before names were restricted to cut the path short. + return await request( + { ...OpenAPI, ENCODE_PATH: encodeURIComponent }, + { + method: 'GET', + url: '/w/{workspace}/workspaces/datatable_usable_roles/{datatable_name}', + path: { workspace, datatable_name: datatableName } + } + ) } catch (e) { const body = (e as { body?: unknown })?.body const detail = `${typeof body === 'string' ? body : JSON.stringify(body ?? '')} ${(e as Error)?.message ?? e}` diff --git a/frontend/src/lib/components/dbOps.ts b/frontend/src/lib/components/dbOps.ts index 02c26f034f..e239a9e957 100644 --- a/frontend/src/lib/components/dbOps.ts +++ b/frontend/src/lib/components/dbOps.ts @@ -299,7 +299,7 @@ export function dbSchemaOpsWithPreviewScripts({ ? input.resourcePath.slice('datatable://'.length) : undefined // A migration declaring no role runs as admin, whatever role the manager connects as. - const migrationRole = input.type === 'database' ? input.role : undefined + const migrationRole = input.type === 'database' ? (input.role ?? input.migrationRole) : undefined function makeMarker(op: string, payload: Record): string { if (ducklake) payload.ducklake = ducklake diff --git a/frontend/src/lib/components/dbTypes.ts b/frontend/src/lib/components/dbTypes.ts index a9cb0a81e1..e6d3af12e8 100644 --- a/frontend/src/lib/components/dbTypes.ts +++ b/frontend/src/lib/components/dbTypes.ts @@ -6,6 +6,9 @@ export type DbInput = /** The data table role to connect as; the data table's default when unset. Only * meaningful for a `datatable://` path. */ role?: string + /** The role migrations written through this input declare when `role` is unset. A + * migration declaring none runs as admin, not as the role the manager connects as. */ + migrationRole?: string specificSchema?: string specificTable?: string } @@ -42,6 +45,16 @@ export function datatableNameTakesRole(name: string): boolean { return !name.includes('?') } +/** The `migrationRole` of a data table that cannot name a role in its reference: it connects as + * its default role, which its migrations must then declare. */ +export function defaultMigrationRole( + name: string, + permissioned: boolean | undefined, + defaultRole: string | undefined +): string | undefined { + return permissioned && !datatableNameTakesRole(name) ? defaultRole : undefined +} + /** `datatable://`, with `?role=` when a role is named. Throws rather than build a * reference the executor would refuse, or one that would silently mean another role. */ export function datatableReference(name: string, role: string | undefined): string { diff --git a/frontend/src/lib/components/details/DetailPageHeader.svelte b/frontend/src/lib/components/details/DetailPageHeader.svelte index 77c802dcd2..4f91f1800f 100644 --- a/frontend/src/lib/components/details/DetailPageHeader.svelte +++ b/frontend/src/lib/components/details/DetailPageHeader.svelte @@ -137,7 +137,7 @@
diff --git a/frontend/src/lib/components/details/DetailPageLayout.svelte b/frontend/src/lib/components/details/DetailPageLayout.svelte index be6e1a9e79..a6a2aa7a8a 100644 --- a/frontend/src/lib/components/details/DetailPageLayout.svelte +++ b/frontend/src/lib/components/details/DetailPageLayout.svelte @@ -1,5 +1,6 @@
@@ -54,7 +74,7 @@
- {@render form?.()} + {@render form?.({ graphInline: true })} @@ -65,7 +85,11 @@ {@render save_inputs_render?.()} {/snippet} {#snippet flow_step()} - {@render flow_step_render?.()} + +
+ {@render flow_step_render?.({})} +
{/snippet} {#snippet triggers()} {@render triggers_render?.()} @@ -79,12 +103,15 @@
{@render header?.()}
- + + {#if !isChatMode} {/if} - {#if isChatMode && flow_json} + {#if flow_json} {/if} {#if !isOperator} @@ -99,7 +126,7 @@ {#snippet content()}
- {@render form?.()} + {@render form?.({ graphInline: false })} @@ -108,9 +135,9 @@ {@render triggers?.()} - {#if isChatMode && flow_json} + {#if flow_json} - {@render flow_graph_render?.()} + {@render pagedGraph()} {/if} {#if flow_json} @@ -128,3 +155,34 @@
{/if}
+ + +{#snippet pagedGraph()} + { + if (key === 'graph') selected = 'saved_inputs' + }} + pages={[ + { key: 'graph', content: graphPageContent }, + { key: 'step', content: stepPageContent } + ]} + /> +{/snippet} + +{#snippet graphPageContent()} +
+ {@render flow_graph_render?.()} +
+{/snippet} + +{#snippet stepPageContent()} + +
+ {@render flow_step_render?.({ onBack: () => (selected = 'saved_inputs') })} +
+{/snippet} diff --git a/frontend/src/lib/components/flows/content/FlowInput.svelte b/frontend/src/lib/components/flows/content/FlowInput.svelte index 3456b4ead6..eb78dbdc89 100644 --- a/frontend/src/lib/components/flows/content/FlowInput.svelte +++ b/frontend/src/lib/components/flows/content/FlowInput.svelte @@ -26,7 +26,8 @@ Save, X, Check, - Settings2 + Settings2, + MessageSquare } from 'lucide-svelte' import CaptureIcon from '$lib/components/triggers/CaptureIcon.svelte' import FlowInputEditor from './FlowInputEditor.svelte' @@ -47,9 +48,12 @@ import type { AiAgent, InputTransform, ScriptLang } from '$lib/gen' import { deepEqual } from 'fast-equals' import Toggle from '$lib/components/Toggle.svelte' + import ToggleButtonGroup from '$lib/components/common/toggleButton-v2/ToggleButtonGroup.svelte' + import ToggleButton from '$lib/components/common/toggleButton-v2/ToggleButton.svelte' import { AI_AGENT_SCHEMA } from '../flowInfers' import { nextId } from '../flowModuleNextId' - import ConfirmationModal from '$lib/components/common/confirmationModal/ConfirmationModal.svelte' + import { fetchAgentWithDraft, normalizeAgentRef } from '../linkedAgentDrafts' + import type { AIAgentConfig } from '../agentResourceUtils' import FlowChat from '../conversations/FlowChat.svelte' import { SPECIAL_MODULE_IDS } from '$lib/components/copilot/chat/shared' @@ -96,9 +100,10 @@ ) let chatInputEnabled = $state(Boolean(flowStore.val.value?.chat_input_enabled)) - let showChatModeWarning = $state(false) - let showAdditionalInputs = $state(false) + // Chat mode shows one of the two at a time: the conversation, or the inputs it sends. + let chatPanelTab = $state<'chat' | 'inputs'>('chat') let chatInputsEditTab = $state(false) + let chatEditableSchemaForm: EditableSchemaForm | undefined = $state(undefined) let chatInputsAddPropertyV2: AddPropertyV2 | undefined = $state(undefined) let addPropertyV2: AddPropertyV2 | undefined = $state(undefined) @@ -511,23 +516,9 @@ return jobId } - function hasOtherInputs(): boolean { - const properties = flowStore.val.schema?.properties - return Boolean( - properties && - Object.keys(properties).length > 0 && - !(Object.keys(properties).length === 1 && Object.keys(properties).includes('user_message')) - ) - } - function handleToggleChatMode() { if (!flowStore.val.value?.chat_input_enabled) { - // Check if there are existing inputs - if (hasOtherInputs()) { - showChatModeWarning = true - } else { - enableChatMode() - } + enableChatMode() } else { // Disable chat input - remove from flow.value if (flowStore.val.value) { @@ -536,21 +527,77 @@ } } + /** + * Add the flow input the agent's `user_attachments` reads, and return its name. Chat + * mode means files dropped in the composer, and that only works through a flow input — + * so it is set up with the message and the memory rather than left to be discovered. + */ + function addAttachmentsInput(): string { + const schema = (flowStore.val.schema ?? {}) as Record + const properties: Record = (schema.properties ??= {}) + let name = 'files' + for (let i = 2; name in properties; i++) name = `files_${i}` + properties[name] = { + type: 'array', + items: { type: 'object', resourceType: 's3object' }, + description: 'Images or PDFs for the agent to read' + } + flowStore.val.schema = schema + return name + } + + // The step panel seeds an array-typed field with [] and undefined persists as null + // through JSON round-trips, so both count as nothing configured. + function isEmptyAgentChatInputValue(value: unknown): boolean { + return value == null || value === '' || (Array.isArray(value) && value.length === 0) + } + + /** + * Memory and streaming of a linked agent are the resource's, so enabling chat mode cannot + * set them. Read the agent, its draft first, and say what the chat will lack. + */ + async function warnIfLinkedAgentCannotChat(path: string) { + if (!opWs) return + let args: AIAgentConfig + try { + const { response, draft } = await fetchAgentWithDraft(path, opWs) + args = draft?.args ?? ((response.value ?? {}) as AIAgentConfig) + } catch { + // Unreadable here means unreadable at run time too; that run reports it. + return + } + const missing: string[] = [] + if (!args.memory || (args.memory as { kind?: string }).kind === 'off') missing.push('memory') + if (args.streaming !== true) missing.push('streaming') + if (missing.length > 0) { + sendUserToast( + `The linked agent ${path} has ${missing.join(' and ')} off. The chat needs both; turn them on in the agent.`, + true + ) + } + } + function enableChatMode() { // Enable chat input - set in flow.value flowStore.val.value.chat_input_enabled = true - // Set up the schema for chat input + // The chat fills the flow's form rather than standing in for it: all it needs is a + // `user_message` string, which the server requires under that exact argument name. + // Every other input stays — the composer drives the ones an agent field reads, and + // the rest are asked for in the Configure-inputs modal. + const schema = flowStore.val.schema ?? {} + const properties = { ...(schema.properties ?? {}) } + // Only a string can carry the message; anything else here cannot be what chat sends. + if (properties['user_message']?.type !== 'string') { + properties['user_message'] = { type: 'string', description: 'Message from user' } + } + const required: string[] = Array.isArray(schema.required) ? schema.required : [] flowStore.val.schema = { $schema: 'https://json-schema.org/draft/2020-12/schema', + ...schema, type: 'object', - properties: { - user_message: { - type: 'string', - description: 'Message from user' - } - }, - required: ['user_message'] + properties, + required: required.includes('user_message') ? required : [...required, 'user_message'] } // Find all AI agent modules @@ -570,8 +617,12 @@ (accu, key) => { if (key === 'user_message') { accu[key] = { type: 'javascript', expr: 'flow_input.user_message' } + } else if (key === 'user_attachments') { + accu[key] = { type: 'javascript', expr: `flow_input.${addAttachmentsInput()}` } } else if (key === 'memory') { accu[key] = { type: 'static', value: { kind: 'auto', context_length: 10 } } + } else if (key === 'streaming') { + accu[key] = { type: 'static', value: true } } else { accu[key] = { type: 'static', @@ -586,25 +637,23 @@ } ] sendUserToast( - 'Chat mode enabled. AI agent created with user message input and context memory set to 10.', + 'Chat mode enabled. AI agent created with user message and attachments inputs, context memory set to 10 and streaming turned on.', false ) } else if (aiAgentModules.length === 1) { // Exactly one AI agent exists: fill in defaults only for inputs the // user hasn't configured, so re-enabling chat mode on an already - // configured agent doesn't clobber a custom user_message expression - // or a deliberate memory choice (e.g. off). + // configured agent doesn't clobber a custom user_message expression. const aiAgent = aiAgentModules[0] const value = aiAgent.value as AiAgent // Degenerate shapes the input form can produce without deliberate - // configuration count as unconfigured: empty static value (undefined - // persists as null through JSON round-trips), blank JS expression - // (the JS toggle seeds a bare backtick pair), or an AI transform - // (meaningless for the chat input). + // configuration count as unconfigured: empty static value, blank JS + // expression (the JS toggle seeds a bare backtick pair), or an AI + // transform (meaningless for the chat input). const isUnconfigured = (transform: InputTransform | undefined) => transform === undefined || - (transform.type === 'static' && (transform.value == null || transform.value === '')) || + (transform.type === 'static' && isEmptyAgentChatInputValue(transform.value)) || (transform.type === 'javascript' && transform.expr.replaceAll('`', '').trim() === '') || transform.type === 'ai' @@ -617,12 +666,47 @@ applied.push('user message input') } - if (isUnconfigured(value.input_transforms['memory'])) { - value.input_transforms['memory'] = { - type: 'static', - value: { kind: 'auto', context_length: 10 } + if (isUnconfigured(value.input_transforms['user_attachments'])) { + value.input_transforms['user_attachments'] = { + type: 'javascript', + expr: `flow_input.${addAttachmentsInput()}` + } + applied.push('attachments input') + } + + // A linked step's brain lives in the agent resource: the worker overlays only the + // flow-local keys from the step, so a memory or streaming transform written here + // would be ignored. Those are checked on the agent itself below instead. + const linkedAgent = value.agent ? normalizeAgentRef(value.agent) : undefined + if (linkedAgent === undefined) { + // `off` is the first oneOf variant of the memory field, so a step added by hand + // carries it without anyone choosing it — and an agent that forgets every turn + // makes the chat a series of unrelated questions. Overwritten rather than left + // alone; the toast below says it happened. + const memoryIsOff = (transform: InputTransform | undefined) => + transform?.type === 'static' && (transform.value as any)?.kind === 'off' + + if ( + isUnconfigured(value.input_transforms['memory']) || + memoryIsOff(value.input_transforms['memory']) + ) { + value.input_transforms['memory'] = { + type: 'static', + value: { kind: 'auto', context_length: 10 } + } + applied.push('context memory set to 10') + } + + // Without streaming the chat has no SSE to read, so a turn shows nothing — + // no thinking, no answer — until the run ends and its rows are written. + if ( + isUnconfigured(value.input_transforms['streaming']) || + (value.input_transforms['streaming']?.type === 'static' && + value.input_transforms['streaming'].value === false) + ) { + value.input_transforms['streaming'] = { type: 'static', value: true } + applied.push('streaming turned on') } - applied.push('context memory set to 10') } sendUserToast( @@ -631,42 +715,53 @@ : 'Chat mode enabled. Existing AI agent configuration kept unchanged.', false ) + if (linkedAgent !== undefined) { + warnIfLinkedAgentCannotChat(linkedAgent) + } } // If there are multiple AI agents, don't auto-configure (ambiguous which one to configure) - - showChatModeWarning = false } - { - showChatModeWarning = false - chatInputEnabled = false - }} -> -

- Enabling Chat Mode will replace all existing flow inputs with a single - user_message - parameter. -

-

- Your current input configuration will be lost. Are you sure you want to continue? -

- + +{#snippet inputsEditButton(open: boolean, toggle: () => void)} + + + {#snippet children({ item })} + + + {/snippet} + {/if}
{/if} @@ -696,11 +796,15 @@
{#if flowStore.val.value?.chat_input_enabled}
- {#if showAdditionalInputs} -
+ {#if chatPanelTab === 'inputs'} + +
{#snippet openEditTab()} - + {@render inputsAddTrigger()} {/snippet} {/snippet}
{/if} - + +
+ +
{:else}
@@ -815,22 +918,9 @@
{#snippet close_button()} -
@@ -884,12 +974,7 @@ }} > {#snippet trigger()} -
- -
+ {@render inputsAddTrigger()} {/snippet} {/if} diff --git a/frontend/src/lib/components/flows/conversations/FlowChat.svelte b/frontend/src/lib/components/flows/conversations/FlowChat.svelte index 160af2bf27..1025ddaa15 100644 --- a/frontend/src/lib/components/flows/conversations/FlowChat.svelte +++ b/frontend/src/lib/components/flows/conversations/FlowChat.svelte @@ -6,6 +6,7 @@ import FlowChatInterface from './FlowChatInterface.svelte' import { getContext } from 'svelte' import type { FlowEditorContext } from '../types' + import type { FlowModule } from '$lib/gen' interface Props { /** @@ -22,6 +23,11 @@ path: string hideSidebar?: boolean inputSchema?: Record + /** The flow's modules, read for the provider wiring of its AI agent steps. */ + flowModules?: FlowModule[] + /** The flow's description, shown under the empty transcript's prompt. */ + description?: string + wideLayout?: boolean } let { @@ -29,7 +35,10 @@ deploymentInProgress = false, path, hideSidebar = false, - inputSchema = undefined + inputSchema = undefined, + flowModules = undefined, + description = undefined, + wideLayout = false }: Props = $props() const flowEditorContext = getContext('FlowEditorContext') @@ -90,13 +99,19 @@ {#if !hideSidebar} {/if} - + + {#key chat} + + {/key} {/if}
diff --git a/frontend/src/lib/components/flows/conversations/FlowChatInterface.svelte b/frontend/src/lib/components/flows/conversations/FlowChatInterface.svelte index 22fe037728..f6aa0670c4 100644 --- a/frontend/src/lib/components/flows/conversations/FlowChatInterface.svelte +++ b/frontend/src/lib/components/flows/conversations/FlowChatInterface.svelte @@ -1,79 +1,50 @@ - -{#if additionalInputsSchema} +{#if modalSchema} @@ -164,82 +216,81 @@ {/if} -
- -
- {#if deploymentInProgress} - - {/if} - {#if chatState.loadingMessages && chatState.messages.length === 0} -
- -
- {:else if chatState.messages.length === 0} -
- -

Start a conversation

-

Send a message to run the flow and see the results

-
+{#snippet emptyHint()} +
+ {#if chatHost.state.loadingMessages} + {:else} -
- {#each chatState.messages as message (message.id)} - - {/each} - {#if busy} -
- - Processing... -
- {/if} -
- {/if} -
- - -
- {#if additionalInputsSchema} -
-
- - {#if hasMissingRequired} - - {/if} + +

Start a conversation

+

Send a message to run the flow and see the results

+ {#if !emptyString(description)} +
+
-
+ {/if} {/if} -
- { - if (e.key === 'Enter' && !e.shiftKey && !e.isComposing) { - e.preventDefault() - handleSendMessage() - } - }} - showCancelButton={busy} - onCancel={() => chat.stop()} - sendTitle={deploymentInProgress ? 'Deployment in progress' : 'Send message (Enter)'} - /> -
+{/snippet} + +{#snippet footerSettings()} + {#if modalSchema} +
+ + {#if modalMissingRequired} + + {/if} +
+ {/if} + {#if modelWiring && showModelButton} + + + {/if} +{/snippet} + + +
0} + class:min-h-64={chatHost.displayMessages.length === 0} +> + {}} + deletePastChat={() => {}} + saveAndClear={() => {}} + />
diff --git a/frontend/src/lib/components/flows/conversations/FlowChatModelSettings.svelte b/frontend/src/lib/components/flows/conversations/FlowChatModelSettings.svelte new file mode 100644 index 0000000000..f27e14749e --- /dev/null +++ b/frontend/src/lib/components/flows/conversations/FlowChatModelSettings.svelte @@ -0,0 +1,299 @@ + + +{#snippet providerSummary()} + {#if resourcePath} + {resourcePath} + {/if} +{/snippet} + +{#if resourceEditable} + { + resourcesVersion++ + if (e.detail) { + pendingResourcePath = e.detail + } + }} + /> +{/if} + + diff --git a/frontend/src/lib/components/flows/conversations/agentChatInputs.test.ts b/frontend/src/lib/components/flows/conversations/agentChatInputs.test.ts new file mode 100644 index 0000000000..c2732353cb --- /dev/null +++ b/frontend/src/lib/components/flows/conversations/agentChatInputs.test.ts @@ -0,0 +1,492 @@ +import { describe, expect, it } from 'vitest' +import { + agentModelGap, + agentModelWiringInputs, + composerOwnedInputs, + parseProviderTransform, + resolveAgentModelWiring, + showsModelButton, + withoutRejectedEffort +} from './agentChatInputs' +import type { FlowModule } from '$lib/gen' + +function agent(expr: string): FlowModule { + return { + id: 'a', + value: { + type: 'aiagent', + tools: [], + input_transforms: { provider: { type: 'javascript', expr } } + } + } as unknown as FlowModule +} + +describe('parseProviderTransform', () => { + it('reads a whole-object reference', () => { + expect(parseProviderTransform({ type: 'javascript', expr: 'flow_input.model' })).toEqual({ + whole: 'model', + fields: {}, + fixed: {} + }) + }) + + it('splits a partial expression into wired and fixed fields', () => { + const wiring = parseProviderTransform({ + type: 'javascript', + expr: "{ kind: 'anthropic', resource: '$res:u/admin/claude', model: flow_input.model, reasoning_effort: flow_input.thinking }" + }) + expect(wiring).toEqual({ + fields: { model: 'model', reasoning_effort: 'thinking' }, + fixed: { kind: 'anthropic', resource: '$res:u/admin/claude' } + }) + }) + + // The regression this detector exists for: counting flow_input references would read + // this as one input carrying the whole provider object, and the composer would write + // {kind, resource, model} into an input the expression uses as the model name. + it('does not mistake a single-reference partial expression for a whole-object one', () => { + const wiring = parseProviderTransform({ + type: 'javascript', + expr: "{ kind: 'anthropic', resource: '$res:u/admin/claude', model: flow_input.model }" + }) + expect(wiring?.whole).toBeUndefined() + expect(wiring?.fields).toEqual({ model: 'model' }) + }) + + // The flow editor's JS field commonly holds a parenthesised object, which is how an + // author writes one without it reading as a block. + it('accepts a parenthesised object expression', () => { + const wiring = parseProviderTransform({ + type: 'javascript', + expr: `({ + "kind": "anthropic", + "resource": "$res:u/admin/anthropic_windmill_codegen", + "model": "claude-sonnet-5", + "reasoning_effort": flow_input.thinking +})` + }) + expect(wiring).toEqual({ + fields: { reasoning_effort: 'thinking' }, + fixed: { + kind: 'anthropic', + resource: '$res:u/admin/anthropic_windmill_codegen', + model: 'claude-sonnet-5' + } + }) + }) + + it('treats a static provider as entirely fixed', () => { + expect( + parseProviderTransform({ + type: 'static', + value: { kind: 'openai', resource: '$res:u/admin/oai', model: 'gpt-5.6' } + }) + ).toEqual({ + fields: {}, + fixed: { kind: 'openai', resource: '$res:u/admin/oai', model: 'gpt-5.6' } + }) + }) + + it.each([ + ['{ ...base, model: flow_input.model }', 'a spread could supply any field'], + ['{ model: pickModel(flow_input.x) }', 'a call is not classifiable'], + ['flow_input.model + 1', 'not a bare reference'], + ['{ model: ', 'unparseable'] + ])('gives up on %s', (expr) => { + expect(parseProviderTransform({ type: 'javascript', expr } as any)).toBeUndefined() + }) +}) + +describe('agentModelWiringInputs', () => { + // The button writes `kind` only alongside a resource, since a provider is picked as a + // pair. Hiding a kind input it cannot write would leave the run without one. + it('keeps a kind input the model button cannot write', () => { + const wiring = resolveAgentModelWiring([ + agent(`({ kind: flow_input.k, "resource": "$res:u/admin/claude", model: flow_input.m })`) + ]) + expect(agentModelWiringInputs(wiring)).toEqual(['m']) + }) + + it('hides a kind input it writes with the resource', () => { + const wiring = resolveAgentModelWiring([ + agent(`({ kind: flow_input.k, resource: flow_input.r, model: flow_input.m })`) + ]) + expect(agentModelWiringInputs(wiring)?.sort()).toEqual(['k', 'm', 'r']) + }) + + // A thinking control is usable whatever the registry knows about the model — a ladder, a + // typed token, or why there is neither — so a wired effort is the button's there. + it('claims a wired reasoning_effort whatever the provider', () => { + const custom = resolveAgentModelWiring([ + agent( + `({ "kind": "customai", "resource": "$res:u/admin/custom", model: flow_input.m, reasoning_effort: flow_input.thinking })` + ) + ]) + expect(agentModelWiringInputs(custom)?.sort()).toEqual(['m', 'thinking']) + + const known = resolveAgentModelWiring([ + agent( + `({ "kind": "openai", "resource": "$res:u/admin/oai", "model": "gpt-4o", reasoning_effort: flow_input.thinking })` + ) + ]) + expect(agentModelWiringInputs(known)).toEqual(['thinking']) + }) + + // Agents on different fixed models leave the button no model to place the effort on, so + // it would say "Pick a model first" with nothing to pick. The modal keeps the input. + it('leaves a shared effort to the modal when the agents fix different models', () => { + const resource = `"kind": "anthropic", "resource": "$res:u/admin/claude"` + const wiring = resolveAgentModelWiring([ + agent(`({ ${resource}, "model": "claude-sonnet-5", reasoning_effort: flow_input.thinking })`), + agent(`({ ${resource}, "model": "claude-opus-5", reasoning_effort: flow_input.thinking })`) + ]) + expect(wiring?.fields.reasoning_effort).toBe('thinking') + expect(agentModelWiringInputs(wiring)).toEqual([]) + expect(showsModelButton(wiring)).toBe(false) + }) + + // The same one level up: agents on different providers leave the model menu nothing to + // list and no provider to gate a typed id, so the shared model input stays in the modal. + it('leaves a shared model to the modal when the agents fix different providers', () => { + const wiring = resolveAgentModelWiring([ + agent(`({ "kind": "openai", "resource": "$res:u/admin/oai", model: flow_input.model })`), + agent( + `({ "kind": "azure_openai", "resource": "$res:u/admin/azure", model: flow_input.model })` + ) + ]) + expect(wiring?.fields.model).toBe('model') + expect(agentModelWiringInputs(wiring)).toEqual([]) + expect(showsModelButton(wiring)).toBe(false) + }) +}) + +// The modal is whatever this does not return, so the two cannot disagree about an input. +describe('composerOwnedInputs', () => { + const wiring = () => + resolveAgentModelWiring([ + agent(`({ "kind": "openai", "resource": "$res:u/admin/oai", model: flow_input.m })`) + ]) + + it('claims the model wiring and the attachments target together', () => { + expect(composerOwnedInputs(wiring(), { name: 'files' })?.sort()).toEqual(['files', 'm']) + }) + + it('claims the attachments input whatever the workspace can do with it', () => { + // No object storage is a state the paperclip shows, not a handover: the modal could + // not upload either, and the run would fail fetching a key typed there. + expect(composerOwnedInputs(undefined, { name: 'files' })).toEqual(['files']) + }) + + it('leaves an input no control fits to the modal', () => { + expect(composerOwnedInputs(wiring(), undefined)).toEqual(['m']) + }) +}) + +describe('resolveAgentModelWiring', () => { + const fixedResource = `"kind": "anthropic", "resource": "$res:u/admin/claude"` + + it('drives a field every agent reads from the same input', () => { + const wiring = resolveAgentModelWiring([ + agent( + `({ ${fixedResource}, "model": "claude-sonnet-5", reasoning_effort: flow_input.thinking })` + ), + agent( + `({ ${fixedResource}, "model": "claude-opus-5", reasoning_effort: flow_input.thinking })` + ) + ]) + expect(wiring?.fields).toEqual({ reasoning_effort: 'thinking' }) + // The agents run different models, so there is no single one to name. + expect(wiring?.fixed).toEqual({ kind: 'anthropic', resource: '$res:u/admin/claude' }) + }) + + it('drops a field the agents disagree about', () => { + const wiring = resolveAgentModelWiring([ + agent(`({ ${fixedResource}, reasoning_effort: flow_input.thinking })`), + agent(`({ ${fixedResource}, reasoning_effort: flow_input.other })`) + ]) + expect(wiring?.fields.reasoning_effort).toBeUndefined() + }) + + // A nested agent is the parent agent's tool, not a step the reader is talking to: the + // graph walks it as a child module, and counting it here would defeat the chat's own + // model control (this is the shape of the all-tools example flow). + it("ignores an agent carried as another agent's tool", () => { + const parent = agent('flow_input.model') + ;(parent.value as any).tools = [ + { + id: 'summarize', + value: { + tool_type: 'flowmodule', + type: 'aiagent', + tools: [], + input_transforms: { + provider: { type: 'static', value: { kind: 'anthropic', model: 'claude-sonnet-5' } } + } + } + } + ] + expect(resolveAgentModelWiring([parent])).toEqual({ + whole: 'model', + fields: {}, + fixed: {}, + someAgentCannotRun: false + }) + }) + + it('finds an agent inside a loop or a branch', () => { + const inner = agent(`({ ${fixedResource}, model: flow_input.model })`) + const loop = { + id: 'loop', + value: { type: 'forloopflow', modules: [inner] } + } as unknown as FlowModule + const branch = { + id: 'branch', + value: { type: 'branchone', default: [], branches: [{ modules: [inner] }] } + } as unknown as FlowModule + expect(resolveAgentModelWiring([loop])?.fields.model).toBe('model') + expect(resolveAgentModelWiring([branch])?.fields.model).toBe('model') + }) + + // The control writes one flow input; an agent that fixes the field instead never reads + // it, so offering the control would move one agent and leave the other where it was. + it('does not offer a field one agent wires and another fixes', () => { + const wiring = resolveAgentModelWiring([ + agent(`({ ${fixedResource}, model: flow_input.model })`), + agent(`({ ${fixedResource}, "model": "claude-opus-5" })`) + ]) + expect(wiring?.fields.model).toBeUndefined() + expect(wiring?.fixed.model).toBeUndefined() + }) + + // Disagreeing about the model is not the same as having no model: the flow runs, on a + // different one per agent, and the composer has nothing to fix. + it('says nothing about a model the agents merely disagree about', () => { + const wiring = resolveAgentModelWiring([ + agent(`({ ${fixedResource}, "model": "claude-sonnet-5" })`), + agent(`({ ${fixedResource}, "model": "claude-opus-5" })`) + ]) + expect(agentModelGap(wiring)).toBeUndefined() + }) + + it('still reports an agent with nothing to call', () => { + expect( + agentModelGap(resolveAgentModelWiring([agent(`({ "kind": "openai", "model": "" })`)])) + ).toBe('Pick a provider and model on the AI agent step to use this chat.') + }) + + // An expression the parser cannot account for could supply anything, so the agents it + // belongs to cannot be spoken for either. + it("offers nothing when one agent's provider cannot be read", () => { + expect( + resolveAgentModelWiring([ + agent(`({ ${fixedResource}, model: flow_input.model })`), + agent(`({ ...base, model: flow_input.model })`) + ]) + ).toBeUndefined() + }) + + // Disagreement is not the same as absence, but an agent with an empty model still + // cannot run, however well its neighbour is configured. + it('keeps warning when one agent has no model and another does', () => { + const wiring = resolveAgentModelWiring([ + agent(`({ ${fixedResource}, "model": "claude-sonnet-5" })`), + agent(`({ ${fixedResource}, "model": "" })`) + ]) + expect(agentModelGap(wiring)).toBe( + 'Pick a provider and model on the AI agent step to use this chat.' + ) + }) + + it('refuses a flow mixing whole-object and field-by-field wiring', () => { + expect( + resolveAgentModelWiring([ + agent('flow_input.provider'), + agent(`({ ${fixedResource}, model: flow_input.model })`) + ]) + ).toBeUndefined() + }) +}) + +describe('withoutRejectedEffort', () => { + const wiring = (fields: Record, fixed: Record = {}) => + ({ fields, fixed }) as any + + // The live 400 this guards: Anthropic turns any effort into adaptive thinking, which + // Haiku rejects outright ("adaptive thinking is not supported on this model"). + it('drops an effort the chosen model rejects', () => { + const values = { model: 'claude-haiku-4-5-20251001', reasoning_effort: 'high' } + expect( + withoutRejectedEffort( + wiring({ model: 'model', reasoning_effort: 'reasoning_effort' }, { kind: 'anthropic' }), + values + ) + ).toEqual({ model: 'claude-haiku-4-5-20251001', reasoning_effort: '' }) + }) + + it('keeps an effort the model takes', () => { + const values = { model: 'claude-sonnet-5', reasoning_effort: 'high' } + expect( + withoutRejectedEffort( + wiring({ model: 'model', reasoning_effort: 'reasoning_effort' }, { kind: 'anthropic' }), + values + ) + ).toBe(values) + }) + + // Clearing on a guess would override the author's own default. + it('leaves the value alone for a family the registry cannot speak for', () => { + const values = { model: 'some-model', reasoning_effort: 'high' } + expect( + withoutRejectedEffort( + wiring({ model: 'model', reasoning_effort: 'reasoning_effort' }, { kind: 'customai' }), + values + ) + ).toBe(values) + }) + + // A flow that wires `provider` as one object keeps the effort inside it, so reading + // `fields.reasoning_effort` finds nothing and the 400 would go out unchecked. + it('clears the effort inside a whole-object provider input', () => { + const wiring = resolveAgentModelWiring([agent('flow_input.provider')]) + const values = { + provider: { + kind: 'anthropic', + model: 'claude-haiku-4-5-20251001', + reasoning_effort: 'high' + } + } + expect(withoutRejectedEffort(wiring, values)).toEqual({ + provider: { + kind: 'anthropic', + model: 'claude-haiku-4-5-20251001', + reasoning_effort: '' + } + }) + }) + + it('leaves a whole-object provider alone when the model takes the effort', () => { + const wiring = resolveAgentModelWiring([agent('flow_input.provider')]) + const values = { + provider: { kind: 'anthropic', model: 'claude-sonnet-5', reasoning_effort: 'high' } + } + expect(withoutRejectedEffort(wiring, values)).toBe(values) + }) + + // A model that reasons can still refuse a particular token, and the provider answers with a + // 400 on every turn. + it('drops a level or off token a reasoning model does not take', () => { + const openai = wiring({ model: 'model', reasoning_effort: 'effort' }, { kind: 'openai' }) + expect(withoutRejectedEffort(openai, { model: 'gpt-5', effort: 'none' }).effort).toBe('') + expect(withoutRejectedEffort(openai, { model: 'gpt-5.1', effort: 'xhigh' }).effort).toBe('') + const kept = { model: 'gpt-5.1', effort: 'none' } + expect(withoutRejectedEffort(openai, kept)).toBe(kept) + }) +}) + +/** + * The shape agent chat is usually built in: one agent answers the reader, others do work of + * their own in branches. A sub-agent never sees the message, so what it runs on is not a + * setting this conversation has. + */ +describe('agents that do not read the message', () => { + const answerer = (provider: string) => + ({ + id: 'answerer', + value: { + type: 'aiagent', + tools: [], + input_transforms: { + user_message: { type: 'javascript', expr: 'flow_input.user_message' }, + provider: { type: 'javascript', expr: provider } + } + } + }) as unknown as FlowModule + const subAgent = (provider: string, message = "'critique: ' + results.answerer") => + ({ + id: 'critic', + value: { + type: 'aiagent', + tools: [], + input_transforms: { + user_message: { type: 'javascript', expr: message }, + provider: { type: 'javascript', expr: provider } + } + } + }) as unknown as FlowModule + + const wired = `({ kind: 'anthropic', resource: '$res:u/admin/c', model: flow_input.model })` + const fixed = `({ kind: 'anthropic', resource: '$res:u/admin/c', model: 'claude-sonnet-5' })` + + it('keeps the model control when only a sub-agent fixes its own model', () => { + const wiring = resolveAgentModelWiring([answerer(wired), subAgent(fixed)]) + expect(wiring?.fields.model).toBe('model') + }) + + // Two agents both answering the reader still have to agree: either might be the one + // that replies, so a control moving one of them would be a lie about the other. + it('still needs agreement among the agents that do read the message', () => { + const wiring = resolveAgentModelWiring([ + answerer(wired), + subAgent(fixed, 'flow_input.user_message') + ]) + expect(wiring?.fields.model).toBeUndefined() + }) + + // Nothing to scope to means the flow is shaped in some way this cannot read, so every + // agent counts again rather than none. + it('falls back to every agent when none reads the message', () => { + const wiring = resolveAgentModelWiring([subAgent(wired), subAgent(fixed)]) + expect(wiring?.fields.model).toBeUndefined() + }) + + // The editor writes the dot form, but an author may hand-edit either. A shape this does + // not recognise drops that agent out of the unanimity check it should be part of. + it.each([ + ["flow_input['user_message']", 'bracket access'], + ['flow_input?.user_message', 'optional chaining'], + ["'Answer politely: ' + flow_input.user_message", 'embedded in a prompt'], + ['flow_input.user_message + flow_input.tone', 'read alongside another input'], + ['flow_input.user_message // the message', 'a trailing comment'], + ['flow_input.user_message\n// why', 'a comment on its own last line'] + ])('treats %s as reading the message', (message) => { + const wiring = resolveAgentModelWiring([answerer(wired), subAgent(fixed, message as string)]) + expect(wiring?.fields.model).toBeUndefined() + }) + + it.each([ + ['// see flow_input.user_message', 'a mention in a comment'], + ["'flow_input.user_message'", 'a mention in a string'], + ['flow_input.user_message_extra', 'a different input with the same prefix'] + ])('does not treat %s as reading the message', (message) => { + const wiring = resolveAgentModelWiring([answerer(wired), subAgent(fixed, message as string)]) + expect(wiring?.fields.model).toBe('model') + }) +}) + +// An author annotating their own provider must not lose the model control for it: a +// comment beside the expression is not another expression. +describe('parseProviderTransform with comments', () => { + it.each([ + ['flow_input.provider // the one to use', 'a trailing line comment'], + ['flow_input.provider /* the one to use */', 'a trailing block comment'], + ['/* pick one */ flow_input.provider', 'a leading comment'] + ])('reads a whole-object reference despite %s', (expr) => { + expect(parseProviderTransform({ type: 'javascript', expr })?.whole).toBe('provider') + }) + + it('reads an object literal with a comment inside it', () => { + const wiring = parseProviderTransform({ + type: 'javascript', + expr: `({ kind: 'anthropic', /* fixed */ resource: '$res:u/admin/c', model: flow_input.model })` + }) + expect(wiring?.fields.model).toBe('model') + }) + + // The check exists to reject an expression with something else beside it; a second + // expression is still something else. + it('still refuses a second expression beside it', () => { + expect( + parseProviderTransform({ type: 'javascript', expr: 'flow_input.provider, 1' }) + ).toBeUndefined() + }) +}) diff --git a/frontend/src/lib/components/flows/conversations/agentChatInputs.ts b/frontend/src/lib/components/flows/conversations/agentChatInputs.ts new file mode 100644 index 0000000000..85bd023176 --- /dev/null +++ b/frontend/src/lib/components/flows/conversations/agentChatInputs.ts @@ -0,0 +1,441 @@ +import type { AIProvider, FlowModule, InputTransform } from '$lib/gen' +import { explicitOffToken, getReasoningCapability } from '$lib/components/copilot/reasoningRegistry' +import { carriedReasoning } from '$lib/components/copilot/chatModelSettings' +import { parseExpressionAt } from 'acorn' + +/** + * The flow's own AI agent steps, including those inside loops and branches but never one + * carried as another agent's tool. + * + * The graph walks an agent's tools as if they were child steps (flowTree.ts), which is + * right for the graph and wrong here: a tool agent's provider belongs to the agent that + * calls it, not to the chat. Counting it would let a nested agent's fixed model defeat the + * composer's model control on the step the reader is actually talking to. + */ +function agentSteps(modules: FlowModule[] | undefined): FlowModule[] { + const found: FlowModule[] = [] + const walk = (mods: FlowModule[]) => { + for (const module of mods) { + const value = module.value as any + if (value?.type === 'aiagent') { + found.push(module) + continue + } + if (value?.type === 'forloopflow' || value?.type === 'whileloopflow') { + walk(value.modules ?? []) + } else if (value?.type === 'branchone') { + walk(value.default ?? []) + for (const branch of value.branches ?? []) walk(branch.modules ?? []) + } else if (value?.type === 'branchall') { + for (const branch of value.branches ?? []) walk(branch.modules ?? []) + } + } + } + walk(modules ?? []) + return found +} + +/** Block and line comments removed, so what is left is only what affects the value. */ +function withoutComments(source: string): string { + return source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/\/\/[^\n]*/g, '') +} + +/** + * An expression wrapped so acorn will read it: parenthesised, because a leading `{` would + * otherwise parse as a block, and on its own line, because a trailing `// comment` would + * otherwise swallow the closing paren and make the whole thing unparseable. + */ +function parenthesised(expr: string): string { + return `(\n${expr}\n)` +} + +/** The flow input the server requires on a chat-enabled flow, and stores as the message. */ +const MESSAGE_INPUT = 'user_message' + +/** + * The agents the reader is talking to: the ones the chat's message is fed to. + * + * A flow commonly runs one agent on the message and others on work of their own — a + * critic reading `results.x`, a classifier in a branch. Those never see what was typed, + * so what they run on is not a setting this conversation has: letting one of them differ + * on the model would take the model control away from the agent that does answer. + * + * The message need not be the whole prompt — an author wraps it in context freely — so + * this asks whether the expression reads it at all. A flow where no agent reads it is one + * shaped in some way this cannot speak for, and every agent counts again rather than none. + */ +function chatFacingAgents(modules: FlowModule[] | undefined): FlowModule[] { + const agents = agentSteps(modules) + const facing = agents.filter((module) => { + const transform = (module.value as any).input_transforms?.[MESSAGE_INPUT] + return transform?.type === 'javascript' && readsFlowInput(transform.expr, MESSAGE_INPUT) + }) + return facing.length > 0 ? facing : agents +} + +/** + * Whether an expression reads `flow_input.` anywhere in it. + * + * Parsed rather than matched: the author may write `flow_input['user_message']` as readily + * as the dot form the editor emits, and a mention inside a comment or a string is not a + * read. Reading two inputs is still a read of each, which is why this is not the question + * "which single input feeds a field" that the composer asks of a wired field. + */ +function readsFlowInput(expr: string, name: string): boolean { + let root: unknown + try { + root = parseExpressionAt(parenthesised(expr), 0, { ecmaVersion: 'latest' }) + } catch { + return false + } + let found = false + const visit = (node: any) => { + if (found || !node || typeof node !== 'object') return + if (Array.isArray(node)) { + node.forEach(visit) + return + } + if (flowInputName(node) === name) { + found = true + return + } + for (const key of Object.keys(node)) { + if (key === 'type' || key === 'start' || key === 'end') continue + visit(node[key]) + } + } + visit(root) + return found +} + +/** A provider value as the agent stores it. */ +export type AgentModel = { kind?: string; model?: string; reasoning_effort?: string } + +/** The provider fields the composer can read or drive. */ +const PROVIDER_FIELDS = ['kind', 'resource', 'model', 'reasoning_effort'] as const +export type ProviderField = (typeof PROVIDER_FIELDS)[number] + +/** + * How an AI agent step's `provider` is supplied, field by field. + * + * The agent takes one `provider` object, so an author who wants the chat to choose only + * the model writes the rest as literals around it: + * + * { kind: 'anthropic', resource: '$res:u/admin/claude', model: flow_input.model } + * + * `fields` names the flow input behind each field the author exposed, `fixed` holds the + * literals, and `whole` covers the plain `flow_input.x` case where one input carries the + * entire object. Reading them apart is what lets the composer offer exactly the knobs the + * flow exposed — and stops a partial expression from being mistaken for a whole-object + * one, which would write a provider object into an input the flow reads as a model name. + */ +export type AgentModelWiring = { + whole?: string + fields: Partial> + fixed: Partial> + /** + * One of the agents names no resource or no model of its own and no flow input feeds + * it, so that agent's run fails whatever the others do. Held apart from the fields, + * which describe what the composer may offer. + */ + someAgentCannotRun?: boolean +} + +/** The flow input behind `flow_input.x`, `flow_input?.x` or `flow_input['x']`. */ +function flowInputName(node: any): string | undefined { + const member = node?.type === 'ChainExpression' ? node.expression : node + if (member?.type !== 'MemberExpression') return undefined + if (member.object?.type !== 'Identifier' || member.object.name !== 'flow_input') return undefined + if (!member.computed && member.property?.type === 'Identifier') return member.property.name + if (member.computed && member.property?.type === 'Literal') { + return typeof member.property.value === 'string' ? member.property.value : undefined + } + return undefined +} + +/** A property's name, for the plain `key:` and `'key':` forms only. */ +function propertyKey(property: any): string | undefined { + if (property?.type !== 'Property' || property.computed) return undefined + if (property.key?.type === 'Identifier') return property.key.name + if (property.key?.type === 'Literal' && typeof property.key.value === 'string') { + return property.key.value + } + return undefined +} + +/** + * Read a `provider` input transform. Anything this cannot account for in full returns + * undefined rather than a guess: the composer then leaves the field alone instead of + * writing into an expression it does not understand. + */ +export function parseProviderTransform( + transform: InputTransform | undefined +): AgentModelWiring | undefined { + if (transform?.type === 'static') { + const value = transform.value + if (!value || typeof value !== 'object') return undefined + const fixed: AgentModelWiring['fixed'] = {} + for (const field of PROVIDER_FIELDS) { + if (value[field] !== undefined) fixed[field] = value[field] + } + return { fields: {}, fixed } + } + if (transform?.type !== 'javascript') return undefined + + // The author's own text may already be wrapped, so any balanced surround is fine — what + // the span check rejects is an expression with something else beside it. + const source = parenthesised(transform.expr) + let node: any + try { + node = parseExpressionAt(source, 0, { ecmaVersion: 'latest' }) + } catch { + return undefined + } + // A comment beside the expression is not another expression: the author annotating their + // own provider must not cost them the model control. Dropped before the check so what + // remains is only what would change the value — and so a paren inside a comment is not + // counted as one of the wrapping pair. + const before = withoutComments(source.slice(0, node.start)) + const after = withoutComments(source.slice(node.end)) + if (!/^[\s(]*$/.test(before) || !/^[\s)]*$/.test(after)) return undefined + if ((before.match(/\(/g)?.length ?? 0) !== (after.match(/\)/g)?.length ?? 0)) return undefined + + const whole = flowInputName(node) + if (whole) return { whole, fields: {}, fixed: {} } + if (node.type !== 'ObjectExpression') return undefined + + const fields: AgentModelWiring['fields'] = {} + const fixed: AgentModelWiring['fixed'] = {} + for (const property of node.properties) { + const key = propertyKey(property) + // A spread or a computed key could supply any field, so nothing here is knowable. + if (!key) return undefined + if (!(PROVIDER_FIELDS as readonly string[]).includes(key)) continue + const name = flowInputName(property.value) + if (name) { + fields[key as ProviderField] = name + } else if (property.value?.type === 'Literal') { + fixed[key as ProviderField] = property.value.value + } else { + return undefined + } + } + return { fields, fixed } +} + +/** How one agent supplies a provider field: from an input, as a literal, or not at all. */ +type FieldSupply = + | { kind: 'wired'; name: string } + | { kind: 'fixed'; value: any } + | { kind: 'absent' } + +/** Whether one agent supplies a field with nothing usable: no input, and no literal. */ +function agentFieldEmpty(wiring: AgentModelWiring, field: ProviderField): boolean { + if (wiring.fields[field] !== undefined) return false + const value = wiring.fixed[field] + return value === undefined || value === '' +} + +function fieldSupply(wiring: AgentModelWiring, field: ProviderField): FieldSupply { + const name = wiring.fields[field] + if (name !== undefined) return { kind: 'wired', name } + const value = wiring.fixed[field] + if (value !== undefined) return { kind: 'fixed', value } + return { kind: 'absent' } +} + +/** + * The provider wiring the chat can act on, across every AI agent in the flow. + * + * With several agents a field is drivable when they agree on it: one flow input feeding + * it, or one literal fixing it. Where they disagree there is no single value to show or + * write, so that field is dropped and the others still work. A flow mixing whole-object + * and field-by-field wiring is ambiguous throughout and yields nothing. + */ +export function resolveAgentModelWiring( + modules: FlowModule[] | undefined +): AgentModelWiring | undefined { + const agents = chatFacingAgents(modules) + const parsed = agents.map((agent) => + parseProviderTransform((agent.value as any).input_transforms?.['provider']) + ) + if (parsed.length === 0) return undefined + // An agent whose provider cannot be read is an agent the composer cannot speak for: + // dropping it would let the rest declare a control that governs only some of them. + if (parsed.some((wiring) => wiring === undefined)) return undefined + const wirings = parsed as AgentModelWiring[] + // Whether any single agent has nothing to call, which stays true however the others + // are wired — the gap message is about that agent, not about their agreement. + const someAgentCannotRun = wirings.some( + (wiring) => + !wiring.whole && (agentFieldEmpty(wiring, 'resource') || agentFieldEmpty(wiring, 'model')) + ) + if (wirings.length === 1) return { ...wirings[0], someAgentCannotRun } + + const wholes = new Set(wirings.map((w) => w.whole)) + if (wholes.size === 1 && !wholes.has(undefined)) { + return { whole: [...wholes][0], fields: {}, fixed: {}, someAgentCannotRun } + } + if (wirings.some((w) => w.whole !== undefined)) return undefined + + const fields: AgentModelWiring['fields'] = {} + const fixed: AgentModelWiring['fixed'] = {} + for (const field of PROVIDER_FIELDS) { + // Every agent has to supply the field the same way for the composer to speak for + // them all. One wired name among agents that otherwise fix it is not agreement: + // the control would move that one agent and leave the others where they are. + const supplies = new Set(wirings.map((w) => JSON.stringify(fieldSupply(w, field)))) + // Disagreement leaves the field neither editable nor known: a control offered here + // would govern one agent while the rest ran on something else. + if (supplies.size > 1) continue + const supply: FieldSupply = JSON.parse([...supplies][0]) + if (supply.kind === 'wired') fields[field] = supply.name + else if (supply.kind === 'fixed') fixed[field] = supply.value + } + return { fields, fixed, someAgentCannotRun } +} + +/** + * Why the chat cannot run, when the agent's own provider is incomplete. + * + * A freshly added agent carries `{ kind: 'openai', model: '', resource: '' }`, so it names + * a provider kind while having nothing to call — the run fails and the chat can do nothing + * about it, because no flow input feeds either field. Saying so beats a dead model button. + * A field the flow exposes is never a gap: the reader picks it in the composer. + */ +export function agentModelGap(wiring: AgentModelWiring | undefined): string | undefined { + // No agent, several of them, or an expression we cannot read: not ours to judge. + if (!wiring || wiring.whole) return undefined + // Asked of each agent rather than of what they agree on: agents that merely disagree + // about the model all have one, and the message would be false — while an agent with + // an empty model still cannot run, however well the others are configured. + return wiring.someAgentCannotRun + ? 'Pick a provider and model on the AI agent step to use this chat.' + : undefined +} + +/** + * The provider fields the model button edits. Everything else wired to an input is left to + * the Configure-inputs modal, and the button draws no control for it. + * + * A field is the button's only where its control can be used, which each field makes depend + * on the one before it. An input promoted without a usable control is one nothing can edit. + * - `resource` is always usable: the submenu lists the workspace's AI resources. + * - `kind` is written only alongside a resource, since a provider is picked as a pair. Wired + * with the resource fixed, the button has nothing to write it with. + * - `model` needs a provider to list models for and to gate the typed entry: a wired resource + * or kind, or one fixed kind. Agents that fix different kinds leave none. + * - `reasoning_effort` needs a model to place the effort on: a driven model or one fixed + * model. Agents that fix different models leave none. + */ +export function composerDrivenFields(wiring: AgentModelWiring): Set { + if (wiring.whole) return new Set(PROVIDER_FIELDS) + const wired = (field: ProviderField) => wiring.fields[field] !== undefined + const driven = new Set() + if (wired('resource')) { + driven.add('resource') + if (wired('kind')) driven.add('kind') + } + const providerKnown = wired('resource') || wired('kind') || fixedOne(wiring, 'kind') + if (wired('model') && providerKnown) driven.add('model') + const modelKnown = driven.has('model') || fixedOne(wiring, 'model') + if (wired('reasoning_effort') && modelKnown) driven.add('reasoning_effort') + return driven +} + +/** Whether every agent fixes the field to the same non-empty literal. */ +function fixedOne(wiring: AgentModelWiring, field: ProviderField): boolean { + const value = wiring.fixed[field] + return value !== undefined && value !== '' +} + +/** The flow inputs the model button writes, so the modal does not ask for them twice. */ +export function agentModelWiringInputs(wiring: AgentModelWiring | undefined): string[] { + if (!wiring) return [] + if (wiring.whole) return [wiring.whole] + return [...composerDrivenFields(wiring)] + .map((field) => wiring.fields[field]) + .filter((name): name is string => !!name) +} + +/** + * Whether the composer draws a model button at all: something to write, or a fixed model to + * name. Agents that fix different models leave neither. + */ +export function showsModelButton(wiring: AgentModelWiring | undefined): boolean { + if (!wiring) return false + return agentModelWiringInputs(wiring).length > 0 || fixedOne(wiring, 'model') +} + +/** + * The flow inputs the composer edits, and therefore the ones the Configure-inputs modal must + * not ask for. The modal is whatever is left, so this is the single answer to "who edits + * this" rather than a list kept in step with the controls that render. + * + * Only the shape of the flow decides it. Whether a control can act *right now* — no rules + * for a provider's thinking levels, say — is a state that control shows, not a reason to + * hand the input to an editor that would be no more able. The attachments target is the + * composer's other owner: the input its paperclip uploads into, where the flow has one. + */ +export function composerOwnedInputs( + wiring: AgentModelWiring | undefined, + attachmentsTarget: { name: string } | undefined +): string[] { + return [...agentModelWiringInputs(wiring), ...(attachmentsTarget ? [attachmentsTarget.name] : [])] +} + +/** + * The run's inputs with a reasoning effort the chosen model cannot take removed. + * + * The model button reconciles the two when the reader switches model, which covers the only + * way the composer can put them out of step. It is not the only way they get out of step: + * a value stored from an earlier visit, a default the flow author wrote, or a model chosen + * before the effort was, all arrive already mismatched — and the provider answers a + * mismatch with a 400 that names neither input ("adaptive thinking is not supported on this + * model"). Checked here, where the run's arguments are settled, so every route is covered. + * + * Only where the registry positively knows the model rejects it. An unknown family keeps + * whatever the author wrote: dropping a value on a guess would override their own default. + */ +export function withoutRejectedEffort( + wiring: AgentModelWiring | undefined, + values: Record +): Record { + if (!wiring) return values + + // One input carrying the whole provider object: the three fields are read from it and + // the effort is cleared inside it, since that is where the agent will look for them. + if (wiring.whole) { + const provider = values[wiring.whole] + if (!provider || typeof provider !== 'object') return values + if (!rejectsEffort(provider.kind, provider.model, provider.reasoning_effort)) return values + return { ...values, [wiring.whole]: { ...provider, reasoning_effort: '' } } + } + + const effortInput = wiring.fields.reasoning_effort + if (!effortInput) return values + const kindInput = wiring.fields.kind + const modelInput = wiring.fields.model + const rejected = rejectsEffort( + kindInput ? values[kindInput] : wiring.fixed.kind, + modelInput ? values[modelInput] : wiring.fixed.model, + values[effortInput] + ) + return rejected ? { ...values, [effortInput]: '' } : values +} + +/** + * Whether the registry positively says this model will not take this effort: no reasoning at + * all, a level it does not have (`xhigh` on `gpt-5.1`), or an off token it cannot honour + * (`none` on `gpt-5`). The same rule the button applies when the model changes. + */ +function rejectsEffort(provider: unknown, model: unknown, effort: unknown): boolean { + if (typeof effort !== 'string' || effort === '') return false + if (typeof provider !== 'string' || typeof model !== 'string' || !provider || !model) { + return false + } + const capability = getReasoningCapability(provider as AIProvider, model) + if (!capability.known) return false + const offToken = explicitOffToken(provider as AIProvider, model) + return carriedReasoning(effort, offToken, capability) === undefined +} diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts new file mode 100644 index 0000000000..78118546ac --- /dev/null +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts @@ -0,0 +1,310 @@ +import type { Chat, ChatMessage, ChatState } from 'windmill-chat' +import type { + ChatSendRequestOptions, + ChatViewHost +} from '$lib/components/copilot/chat/chatViewHost' +import type { DisplayMessage } from '$lib/components/copilot/chat/shared' +import type { AIAutonomyMode } from '$lib/components/copilot/chat/AIChatManager.svelte' +import { isPlanCardTool } from '$lib/components/copilot/chat/planMode' +import { AttachedFilesStore } from '$lib/components/copilot/chat/files/attachedFiles.svelte' +import { SessionArtifactsStore } from '$lib/components/copilot/chat/artifacts/artifactsState.svelte' +import type { AttachedImage } from '$lib/components/copilot/chat/imageUtils' +import type { AttachedTextFile } from '$lib/components/copilot/chat/textFileUtils' + +export type FlowChatViewHostOptions = { + /** The flow inputs sent next to `user_message` with every turn. */ + additionalInputs?: () => Record | undefined + /** The workspace the transcript's paths resolve against. */ + workspace?: () => string | undefined + /** Whether sending is refused right now (a deployment in progress, say). The composer + * is disabled on the same condition; this covers the sends the composer does not + * make itself: a queued message going out, a retry. */ + sendDisabled?: () => boolean +} + +function isBusy(status: ChatState['status']): boolean { + return status === 'submitted' || status === 'streaming' +} + +/** A tool's arguments or result as the card shows them: parsed where the string is JSON. */ +function parseToolPayload(raw: string | undefined): unknown { + if (raw === undefined || raw === '') return undefined + try { + return JSON.parse(raw) + } catch { + return raw + } +} + +/** + * Whether the turn the user message at `index` started failed: its last row before the + * next user message reports `success: false`. The last row, not any row: a tool call can + * fail and the agent still answer, and that turn completed. + */ +export function turnFailed(messages: readonly ChatMessage[], index: number): boolean { + let last: ChatMessage | undefined + for (let i = index + 1; i < messages.length; i++) { + const message = messages[i] + if (message.role === 'user') break + last = message + } + return last?.success === false +} + +/** Whether the latest turn failed, per `turnFailed`. False before any turn. */ +function lastTurnFailed(messages: readonly ChatMessage[]): boolean { + for (let i = messages.length - 1; i >= 0; i--) { + if (messages[i].role === 'user') return turnFailed(messages, i) + } + return false +} + +export function toDisplayMessages(messages: readonly ChatMessage[]): DisplayMessage[] { + let userIndex = 0 + return messages.map((message, i): DisplayMessage => { + switch (message.role) { + case 'user': + return { + role: 'user', + index: userIndex++, + content: message.content, + // Drives the shared Retry button. + error: turnFailed(messages, i) || undefined + } + case 'tool': { + const parameters = parseToolPayload(message.tool?.arguments) + const result = parseToolPayload(message.tool?.result) + const failed = message.success === false + return { + role: 'tool', + tool_call_id: message.id, + // The card's header is the row's text, which the server only words once the + // tool has returned; until then the row says what is running. + content: message.content || (message.tool ? `Running ${message.tool.name}` : ''), + // Withheld for the copilot's two plan-mode names: `toolName` is what makes + // ToolExecutionDisplay render a plan card, and an agent tool that happened to + // share one would silently become one. + toolName: isPlanCardTool(message.tool?.name) ? undefined : message.tool?.name, + parameters, + result, + showDetails: parameters !== undefined || result !== undefined, + error: failed ? message.content : undefined, + isLoading: message.pending && message.tool?.status === 'running' + } + } + default: + return { + role: 'assistant', + content: message.content, + // Only the message a turn is still writing: a finalized reasoning-only + // message must not look in progress. + streaming: message.pending || undefined, + reasoning: message.reasoning, + stepName: message.stepName, + jobId: message.jobId, + createdAt: message.createdAt + } + } + }) +} + +/** + * Renders a flow run's conversation, as the `windmill-chat` SDK keeps it, through the + * copilot's chat components. The turn is a flow job rather than an LLM call this host + * makes, so what it can offer is what the SDK's `Chat` can: a message in, an answer + * streamed back, Stop. Every copilot-only field is answered with "no" (see ChatViewHost). + * + * The host owns its subscription to the chat, so a panel swapping chats mounts a new one. + */ +export class FlowChatViewHost implements ChatViewHost { + #chat: Chat + #options: FlowChatViewHostOptions + #state = $state.raw() as ChatState + #unsubscribe: () => void + + constructor(chat: Chat, options: FlowChatViewHostOptions = {}) { + this.#chat = chat + this.#options = options + this.#state = chat.getState() + this.#unsubscribe = chat.subscribe((state) => this.#onState(state)) + } + + #disposed = false + /** Stops following the chat, and drops what was queued: a flush still waiting on the + * turn's release would otherwise start a run from a panel that is gone. The chat itself + * is the caller's to destroy. */ + dispose() { + this.#disposed = true + this.#queued = '' + this.#unsubscribe() + } + + /** The latest `ChatState`, for what the interface reads beyond the seam (paging, loading). */ + get state(): ChatState { + return this.#state + } + + #onState(state: ChatState) { + const previous = this.#state + this.#state = state + if (previous.conversationId !== state.conversationId) { + // A conversation opens at its end, whatever the reader was doing in the last one. + this.#automaticScroll = true + // The queue was typed into the conversation that just went away; a message sent + // after the switch would ride out of the wrong one, so it goes back to the composer. + this.dequeueMessage() + return + } + if (isBusy(previous.status) && !isBusy(state.status)) { + // The turn settled. What was typed during it goes out once the turn is released, + // not now: the chat publishes `idle` from inside its own `sendMessage`, which still + // counts the turn as open until it returns, and a send made before that would be + // refused as a second turn. After a failure it goes back to the composer instead, + // where the reader would rather look at the error than pile on. A failed flow + // settles as `idle` too, with its error as the answer, so the messages decide. + const succeeded = state.status === 'idle' && !lastTurnFailed(state.messages) + if (succeeded) void this.#turnDone.then(this.flushQueuedMessage) + else this.dequeueMessage() + } + } + + // Transcript + displayMessages = $derived.by(() => toDisplayMessages(this.#state.messages)) + get messages(): readonly unknown[] { + return this.#state.messages + } + contextTokens = 0 + get operatingWorkspace(): string | undefined { + return this.#options.workspace?.() + } + get loading(): boolean { + return isBusy(this.#state.status) + } + runHeldElsewhere = false + loadingLabel = undefined + compacting = false + // The answer streams into the message list itself, so the live lanes stay empty. + currentReply = '' + currentReasoning = '' + currentReasoningActive = false + reasoningHiddenIndicatorLabel = undefined + #automaticScroll = $state(true) + get automaticScroll(): boolean { + return this.#automaticScroll + } + enableAutomaticScroll = () => { + this.#automaticScroll = true + } + disableAutomaticScroll = () => { + this.#automaticScroll = false + } + + // Composer + instructions = '' + // The user message lands in the transcript before `sendMessage` awaits anything. + sendInFlight = false + sendRequest = async (options: ChatSendRequestOptions = {}): Promise => { + const text = options.instructions?.trim() ?? '' + if (!text) return false + if (this.loading) { + this.queueMessage(text) + return true + } + if (this.#options.sendDisabled?.()) { + // Refused, not dropped: the text waits in the composer for sending to reopen. + this.#aiChatInput?.prependText(text) + return false + } + this.#automaticScroll = true + // A run that fails is reported through the chat's `onError` and as a failed message; + // the promise itself only rejects when the chat refuses the turn outright, and the + // text is then handed back rather than dropped. + const turn = this.#chat + .sendMessage(text, { inputs: this.#options.additionalInputs?.() }) + .catch(() => this.#aiChatInput?.prependText(text)) + this.#turnDone = turn + await turn + return true + } + /** Settles when the chat has released the last turn this host started. */ + #turnDone: Promise = Promise.resolve() + cancel = () => { + // Stop means stop: what was typed during the run goes back to the composer rather + // than waiting there to go out after some later turn settles. + this.dequeueMessage() + void this.#chat.stop() + } + // Typed off the interface: a Svelte component's own type resolves differently + // across import specifiers, and the two would then not be assignable. + #aiChatInput: Parameters[0] = null + setAiChatInput: ChatViewHost['setAiChatInput'] = (aiChatInput) => { + this.#aiChatInput = aiChatInput + } + + // One message typed while the turn runs, sent whole once it settles. Enter again + // appends a line rather than replacing what waits. + #queued = $state('') + get queuedMessage(): string { + return this.#queued + } + queuedContext = undefined + queuedImages: AttachedImage[] = [] + queuedFiles: AttachedTextFile[] = [] + queueMessage = (text: string) => { + const trimmed = text.trim() + if (!trimmed) return + this.#queued = this.#queued ? `${this.#queued}\n${trimmed}` : trimmed + } + /** Put the queued draft back in the composer. */ + dequeueMessage = () => { + const text = this.#queued + if (!text) return + this.#queued = '' + this.#aiChatInput?.prependText(text) + } + flushQueuedMessage = () => { + const text = this.#queued + if (!text || this.#disposed) return + this.#queued = '' + void this.sendRequest({ instructions: text }) + } + setComposerStaged = () => {} + clearComposerStaged = () => {} + attachmentBytesExcluding = () => 0 + + // Per-message actions + storedImages = () => undefined + /** Send the user message at this transcript position again. */ + retryRequest = (messageIndex: number) => { + const message = this.#state.messages[messageIndex] + if (!message || message.role !== 'user' || this.loading) return + void this.sendRequest({ instructions: message.content }) + } + restartGeneration = () => {} + handleUserQuestionAnswer = () => false + handleToolConfirmation = () => {} + hasPendingRunForm = false + isRunFormPending = () => false + + // Copilot-only surfaces + mode = undefined + isSessionChat = false + supportsModelSettings = false + supportsMessageEditing = false + supportsMessageAttachments = false + // An AI agent step refuses a run with no `user_message`. + requiresMessageText = true + supportsLinkedFolders = false + attachmentAccept = '' + tools = [] + // The enum's value, written out so this module never imports the copilot manager at + // runtime: its unit test would otherwise load the manager and the editor it pulls in. + autonomyMode = 'default' as AIAutonomyMode.DEFAULT + setAutonomyMode = () => {} + autoAcceptEditsActive = false + autoAcceptEditsAvailable = false + autoAcceptToolConfirmationsAvailable = false + planModeAvailable = false + attachedFiles = new AttachedFilesStore() + artifacts = new SessionArtifactsStore() +} diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts new file mode 100644 index 0000000000..a7e953ecf5 --- /dev/null +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts @@ -0,0 +1,267 @@ +import { describe, expect, it, vi } from 'vitest' +import type { Chat, ChatMessage, ChatState } from 'windmill-chat' +import { FlowChatViewHost, toDisplayMessages } from './flowChatViewHost.svelte' + +function message(partial: Partial & Pick): ChatMessage { + return { + id: partial.id ?? `${partial.role}-${Math.random()}`, + content: '', + success: true, + createdAt: '2026-09-16T10:00:00Z', + pending: false, + ...partial + } +} + +function idleState(partial: Partial = {}): ChatState { + return { + conversationId: 'c1', + messages: [], + status: 'idle', + error: undefined, + conversations: [], + history: 'server', + loadingMessages: false, + hasMoreMessages: false, + ...partial + } +} + +/** A `Chat` whose state the test drives by hand. */ +function fakeChat(initial: ChatState = idleState()) { + let state = initial + const listeners = new Set<(s: ChatState) => void>() + const chat = { + getState: () => state, + subscribe: (listener: (s: ChatState) => void) => { + listeners.add(listener) + listener(state) + return () => listeners.delete(listener) + }, + sendMessage: vi.fn(async () => {}), + stop: vi.fn(async () => {}), + newConversation: vi.fn(), + selectConversation: vi.fn(async () => {}), + loadConversations: vi.fn(async () => []), + deleteConversation: vi.fn(async () => {}), + loadOlderMessages: vi.fn(async () => {}), + destroy: vi.fn() + } satisfies Chat + const set = (patch: Partial) => { + state = { ...state, ...patch } + for (const listener of listeners) listener(state) + } + return { chat, set } +} + +describe('toDisplayMessages', () => { + it('maps user, assistant and tool rows, marking the user message of a failed turn', () => { + const rows = [ + message({ role: 'user', content: 'hi' }), + message({ + role: 'assistant', + content: 'hello', + reasoning: 'thinking', + stepName: 'agent', + jobId: 'job-1', + createdAt: '2026-09-16T10:00:01Z' + }), + message({ role: 'user', content: 'again' }), + // A tool still running has no text yet: the server words the row on its return. + message({ + role: 'tool', + id: 'tool-row', + pending: true, + tool: { name: 'search', status: 'running', arguments: '{"q":"x"}' } + }), + message({ role: 'assistant', content: 'boom', success: false }) + ] + const display = toDisplayMessages(rows) + expect(display[0]).toEqual({ role: 'user', index: 0, content: 'hi', error: undefined }) + expect(display[1]).toMatchObject({ + role: 'assistant', + content: 'hello', + reasoning: 'thinking', + stepName: 'agent', + jobId: 'job-1', + createdAt: '2026-09-16T10:00:01Z', + streaming: undefined + }) + expect(display[2]).toEqual({ role: 'user', index: 1, content: 'again', error: true }) + expect(display[3]).toMatchObject({ + role: 'tool', + tool_call_id: 'tool-row', + content: 'Running search', + toolName: 'search', + parameters: { q: 'x' }, + showDetails: true, + isLoading: true, + error: undefined + }) + }) + + it('does not flag a turn whose tool failed but whose agent still answered', () => { + const display = toDisplayMessages([ + message({ role: 'user', content: 'try' }), + message({ + role: 'tool', + content: 'Error executing search', + success: false, + tool: { name: 'search', status: 'error' } + }), + message({ role: 'assistant', content: 'search is down, here is what I know' }) + ]) + expect(display[0]).toMatchObject({ role: 'user', error: undefined }) + }) + + it('flags the streaming assistant message and a failed tool', () => { + const display = toDisplayMessages([ + message({ role: 'assistant', content: 'partial', pending: true }), + message({ + role: 'tool', + content: 'Error executing search', + success: false, + tool: { name: 'search', status: 'error' } + }) + ]) + expect(display[0]).toMatchObject({ role: 'assistant', streaming: true }) + expect(display[1]).toMatchObject({ + role: 'tool', + error: 'Error executing search', + showDetails: false, + isLoading: false + }) + }) +}) + +describe('FlowChatViewHost', () => { + it('sends the text with the additional inputs and reports loading from the status', async () => { + const { chat, set } = fakeChat() + const host = new FlowChatViewHost(chat, { additionalInputs: () => ({ tone: 'brief' }) }) + expect(host.loading).toBe(false) + expect(await host.sendRequest({ instructions: ' hello ' })).toBe(true) + expect(chat.sendMessage).toHaveBeenCalledWith('hello', { inputs: { tone: 'brief' } }) + expect(await host.sendRequest({ instructions: ' ' })).toBe(false) + set({ status: 'streaming' }) + expect(host.loading).toBe(true) + set({ status: 'idle' }) + expect(host.loading).toBe(false) + host.dispose() + }) + + it('queues a message typed during a turn and sends it once the turn is released', async () => { + const { chat, set } = fakeChat() + // The chat publishes `idle` from inside `sendMessage`, before that call returns and + // releases the turn; a flush in between is refused as a second turn. + let releaseTurn = () => {} + chat.sendMessage.mockImplementationOnce( + () => new Promise((resolve) => (releaseTurn = resolve)) + ) + const host = new FlowChatViewHost(chat) + void host.sendRequest({ instructions: 'start' }) + set({ status: 'streaming' }) + host.queueMessage('first') + host.queueMessage('second') + expect(host.queuedMessage).toBe('first\nsecond') + set({ status: 'idle' }) + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(chat.sendMessage).toHaveBeenCalledTimes(1) + releaseTurn() + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(host.queuedMessage).toBe('') + expect(chat.sendMessage).toHaveBeenLastCalledWith('first\nsecond', { inputs: undefined }) + host.dispose() + }) + + it('hands the queue back when a failed flow settles as idle', () => { + const { chat, set } = fakeChat( + idleState({ status: 'streaming', messages: [message({ role: 'user', content: 'go' })] }) + ) + const host = new FlowChatViewHost(chat) + const prependText = vi.fn() + host.setAiChatInput({ prependText } as any) + host.queueMessage('later') + // A failed flow is answered with its error and the status still returns to idle. + set({ + status: 'idle', + messages: [ + message({ role: 'user', content: 'go' }), + message({ role: 'assistant', content: 'boom', success: false }) + ] + }) + expect(prependText).toHaveBeenCalledWith('later') + expect(chat.sendMessage).not.toHaveBeenCalled() + host.dispose() + }) + + it('hands the queue back instead of sending while sending is disabled', async () => { + const { chat, set } = fakeChat(idleState({ status: 'streaming' })) + let deploying = false + const host = new FlowChatViewHost(chat, { sendDisabled: () => deploying }) + const prependText = vi.fn() + host.setAiChatInput({ prependText } as any) + host.queueMessage('after deploy') + // A deployment starts while the turn is still running; the composer is disabled. + deploying = true + set({ status: 'idle' }) + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(chat.sendMessage).not.toHaveBeenCalled() + expect(prependText).toHaveBeenCalledWith('after deploy') + expect(host.queuedMessage).toBe('') + host.dispose() + }) + + it('drops a flush still waiting on the turn once disposed', async () => { + const { chat, set } = fakeChat() + let releaseTurn = () => {} + chat.sendMessage.mockImplementationOnce( + () => new Promise((resolve) => (releaseTurn = resolve)) + ) + const host = new FlowChatViewHost(chat) + void host.sendRequest({ instructions: 'start' }) + set({ status: 'streaming' }) + host.queueMessage('never') + set({ status: 'idle' }) + host.dispose() + releaseTurn() + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(chat.sendMessage).toHaveBeenCalledTimes(1) + }) + + it('hands the text back when the chat refuses the turn', async () => { + const { chat } = fakeChat() + chat.sendMessage.mockRejectedValueOnce(new Error('a message is already being answered')) + const host = new FlowChatViewHost(chat) + const prependText = vi.fn() + host.setAiChatInput({ prependText } as any) + await host.sendRequest({ instructions: 'kept' }) + expect(prependText).toHaveBeenCalledWith('kept') + host.dispose() + }) + + it('hands the queue back to the composer on Stop and on a failed turn', async () => { + const { chat, set } = fakeChat(idleState({ status: 'streaming' })) + const host = new FlowChatViewHost(chat) + const prependText = vi.fn() + host.setAiChatInput({ prependText } as any) + host.queueMessage('later') + host.cancel() + expect(chat.stop).toHaveBeenCalled() + expect(prependText).toHaveBeenCalledWith('later') + expect(host.queuedMessage).toBe('') + + host.queueMessage('after error') + set({ status: 'error' }) + expect(prependText).toHaveBeenLastCalledWith('after error') + expect(chat.sendMessage).not.toHaveBeenCalled() + host.dispose() + }) + + it('stops following the chat once disposed', () => { + const { chat, set } = fakeChat() + const host = new FlowChatViewHost(chat) + host.dispose() + set({ status: 'streaming' }) + expect(host.loading).toBe(false) + }) +}) diff --git a/frontend/src/lib/components/flows/flowInfers.ts b/frontend/src/lib/components/flows/flowInfers.ts index bce923c77a..3c38682350 100644 --- a/frontend/src/lib/components/flows/flowInfers.ts +++ b/frontend/src/lib/components/flows/flowInfers.ts @@ -39,6 +39,12 @@ export const AI_AGENT_SCHEMA: Schema = { }, memory: { type: 'object', + // Chat mode keys memory on the conversation, so a chat whose agent has memory off + // forgets every turn. Enabling chat mode sets `auto`; this keeps it there. A step + // sitting at `off` stays switchable, or a flow that reached that state before — + // an agent added to an already-chat-enabled flow — would have no way out of it. + lockOneOfWhenChatEnabled: + "Chat mode keys this agent's history on the conversation, so memory stays on while it is enabled.", description: 'History sent between the system message and the user message.', oneOf: [ { diff --git a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte index 32a510c54a..291c89c606 100644 --- a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte +++ b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte @@ -114,8 +114,9 @@ try { await gitSyncContext.saveRepository(idx) } catch (e) { - // The backend rejects promotion mode without an active EE plan; revert - // the optimistic toggle instead of leaving it stuck on until reload. + // The backend can reject promotion mode (non-EE build, outdated dev sync + // script, dev repo not matching its parent's); revert the optimistic + // toggle instead of leaving it stuck on until reload. if (repo) { repo.use_individual_branch = prevIndiv repo.group_by_folder = prevGbf diff --git a/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte b/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte index a61bf0a0cb..f0ea7d1ce3 100644 --- a/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte @@ -11,7 +11,12 @@ import { resource } from 'runed' import { ArrowLeft, Expand, Minimize, Plus, RefreshCcw } from 'lucide-svelte' import DBManagerContent from '../DBManagerContent.svelte' - import { ADMIN_DATATABLE_ROLE, datatableNameTakesRole, type DbInput } from '../dbTypes' + import { + ADMIN_DATATABLE_ROLE, + datatableNameTakesRole, + defaultMigrationRole, + type DbInput + } from '../dbTypes' import type { PendingRowAction, SelectedTable } from '../DBManager.svelte' import { getRawAppOperatingWorkspace } from './rawAppWorkspace' import { useDbManagerTag } from '../dbManagerTag.svelte' @@ -107,12 +112,12 @@ // without one runs, and caches, as whatever the server defaults to. const roleSettled = $derived( selectedDatatable === undefined || - // Its reference cannot name a role, so it connects as the default one. - !datatableNameTakesRole(selectedDatatable) || (rolesOfCurrent !== undefined && (!rolesOfCurrent.permissioned || rolesOfCurrent.roles.length === 0 || - selectedRole !== undefined)) + selectedRole !== undefined || + // Its reference cannot name a role, so it connects as the default one. + !datatableNameTakesRole(selectedDatatable))) ) $effect(() => { @@ -307,6 +312,11 @@ resourceType: 'postgresql' as const, resourcePath: `datatable://${selectedDatatable}`, role: selectedRole, + migrationRole: defaultMigrationRole( + selectedDatatable, + rolesOfCurrent?.permissioned, + rolesOfCurrent?.default_role + ), specificSchema: openSchemaKey, specificTable: openTableKey } diff --git a/frontend/src/lib/components/raw_apps/RawAppTemplatePicker.svelte b/frontend/src/lib/components/raw_apps/RawAppTemplatePicker.svelte index 807ec8e9cf..a48713a501 100644 --- a/frontend/src/lib/components/raw_apps/RawAppTemplatePicker.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppTemplatePicker.svelte @@ -34,7 +34,7 @@ toDatatableItems, toSchemaItems } from './datatableUtils.svelte' - import { datatableNameTakesRole } from '../dbTypes' + import { datatableNameTakesRole, defaultMigrationRole } from '../dbTypes' import RawAppDataTableList from './RawAppDataTableList.svelte' import RawAppDataTableDrawer from './RawAppDataTableDrawer.svelte' import FileEditorIcon from './FileEditorIcon.svelte' @@ -166,7 +166,7 @@ rolesSettled && selectedDatatable !== undefined && roles.current.permissioned && - loadedRoles.length === 0 + roles.current.roles.length === 0 ) const rolesUnknown = $derived(rolesSettled && roles.current.failed) const accessSettled = $derived( @@ -290,7 +290,12 @@ type: 'database', resourceType: 'postgresql', resourcePath: `datatable://${selectedDatatable}`, - role: effectiveRole + role: effectiveRole, + migrationRole: defaultMigrationRole( + selectedDatatable, + roles.current.permissioned, + roles.current.defaultRole + ) } }) await dbOps.onCreateSchema({ schema: newSchemaName }) diff --git a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts index 4c70c31ef4..cca8c7cdfb 100644 --- a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts @@ -1240,7 +1240,7 @@ setGetRuntimeLogsHandler(async ({ sessionId: callerSessionId, limit }) => { if (entries.length === 0) { return { aiResult: - 'The raw app preview is running, but it has not emitted console logs, uncaught errors, or unhandled rejections yet. If the user reported a failure, reproduce the interaction in the preview, then call get_app_runtime_logs again. For backend.() failures, call list_app_runs and then get_job_logs for the relevant job_id.', + 'The raw app preview is running, but it has not emitted console logs, uncaught errors, or unhandled rejections yet. If the user reported a failure, reproduce the interaction in the preview, then call get_app_runtime_logs again. For backend.() failures, call list_app_runs and then get_run for the relevant job_id.', uiMessage: 'No runtime logs', toolResult: 'No runtime logs' } diff --git a/frontend/src/lib/components/settings/TokensTable.svelte b/frontend/src/lib/components/settings/TokensTable.svelte index fe85d85f03..abfdd0f36d 100644 --- a/frontend/src/lib/components/settings/TokensTable.svelte +++ b/frontend/src/lib/components/settings/TokensTable.svelte @@ -59,7 +59,10 @@ label !== 'guest_session' && !label.toLowerCase().startsWith('ephemeral') && label !== 'debugger-token' && - !label.startsWith('mcp-oauth-') + !label.startsWith('mcp-oauth-') && + !label.startsWith('embed_app:') && + !label.startsWith('sdk_app:') && + !label.startsWith('impersonation:') ) } diff --git a/frontend/src/routes/(root)/(logged)/flows/get/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/flows/get/[...path]/+page.svelte index 59ed08c8b8..5752fa7e1c 100644 --- a/frontend/src/routes/(root)/(logged)/flows/get/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/flows/get/[...path]/+page.svelte @@ -491,6 +491,7 @@ let stepDetail: FlowModule | string | undefined = $state(undefined) let rightPaneSelected = $state('saved_inputs') let savedInputsV2: SavedInputsV2 | undefined = $state(undefined) + let detailLayout: DetailPageLayout | undefined = $state(undefined) let flowHistory: FlowHistory | undefined = $state(undefined) let path = $derived(page.params.path ?? '') @@ -546,6 +547,7 @@ {/if} { - rightPaneSelected = 'triggers' + detailLayout?.showTriggers() }} {mainButtons} menuItems={getMenuItems(flow, deployUiSettings)} @@ -596,7 +598,7 @@ isFlow selected={rightPaneSelected == 'triggers'} onSelect={async (triggerIndex: number) => { - rightPaneSelected = 'triggers' + detailLayout?.showTriggers() await tick() triggersState.selectedTriggerIndex = triggerIndex }} @@ -627,7 +629,7 @@ {/if} {/snippet} - {#snippet form()} + {#snippet form({ graphInline }: { graphInline: boolean })}
(showEditButtons = v)} />
@@ -701,7 +703,10 @@ onRunFlow={runFlowForChat} {deploymentInProgress} path={flow?.path ?? ''} + description={flow?.description} inputSchema={flow?.schema} + flowModules={flow?.value?.modules} + wideLayout /> {:else} {@const hasSchema = @@ -771,7 +776,7 @@ {/if}
- {#if !chatInputEnabled} + {#if graphInline}
{ - rightPaneSelected = 'triggers' + detailLayout?.showTriggers() }} noBorder={true} /> @@ -817,10 +822,10 @@ /> {/snippet} - {#snippet flow_step()} + {#snippet flow_step({ onBack }: { onBack?: () => void })} {#if flow} {#if stepDetail} - + {/if} {/if} {/snippet} @@ -849,7 +854,7 @@ triggerNode={true} download {flow} - noSide={false} + noSide={true} noBorder minHeight={flowGraphHeight} on:select={(e) => { @@ -862,7 +867,7 @@ } }} on:triggerDetail={(e) => { - rightPaneSelected = 'triggers' + detailLayout?.showTriggers() }} />
diff --git a/system_prompts/auto-generated/flow.md b/system_prompts/auto-generated/flow.md index dd0048ec9c..c204e05e68 100644 --- a/system_prompts/auto-generated/flow.md +++ b/system_prompts/auto-generated/flow.md @@ -102,9 +102,60 @@ tool, `websearch` for web search. } ``` -- `provider` is a static object, not a bare resource string: `{ "kind": , +- `provider` is an object, not a bare resource string: `{ "kind": , "resource": "$res:", "model": }`. Required unless the module links to a saved - agent through `value.agent` + agent through `value.agent`. Static is right for a flow run from a form; a chat flow wires its + fields to flow inputs instead — see below + +### Chat-Mode Flows + +A flow with `value.chat_input_enabled: true` is run from a chat instead of a form: the composer +sends one message per turn and renders the conversation. It needs a required `user_message` string +input, read by the agent. Any other flow input the composer does not edit itself is asked for +under Configure inputs. + +**A static `provider` gives a chat that cannot change its model.** Feed it from flow inputs +instead, either way round: one input carrying the whole object (`"expr": "flow_input.model_config"`) +makes every field editable, or wire it field by field to fix some and expose others. A field the +chat can write becomes a control in the composer — a provider picker, a model list, a thinking +control — and a field left static is fixed, with no control drawn for it. `kind` is the one +exception: the composer writes it only together with `resource`, since a provider is picked as a +pair, so a `kind` input wired on its own stays askable under Configure inputs and nothing the run +needs becomes unreachable. + +```json +{ + "id": "chat_agent", + "value": { + "type": "aiagent", + "input_transforms": { + "provider": { + "type": "javascript", + "expr": "({ kind: 'anthropic', resource: '$res:f/ai/claude', model: flow_input.model, reasoning_effort: flow_input.thinking })" + }, + "user_message": { "type": "javascript", "expr": "flow_input.user_message" }, + "user_attachments": { "type": "javascript", "expr": "flow_input.files" }, + "memory": { "type": "static", "value": { "kind": "auto", "context_length": 10 } }, + "streaming": { "type": "static", "value": true }, + "output_type": { "type": "static", "value": "text" } + }, + "tools": [] + } +} +``` + +- Wiring field by field means one object literal whose values are literals or bare `flow_input.x` + references. A spread, a call or a computed key leaves the composer unable to tell which input + feeds which field, so it offers no control at all — a bare `flow_input.x` for the whole object + is read instead as that one input carrying every field +- `memory` is what lets the agent see earlier turns; without it every message starts from nothing +- `streaming` on makes the answer and its thinking appear token by token instead of all at once +- `user_attachments` points at a flow input typed as an array of s3 objects + (`{ "type": "array", "items": { "type": "object", "resourceType": "s3object" } }`), so files + sent with a message reach the agent +- Running one needs a `memory_id` **query parameter** — not a flow argument — naming the + conversation the turn belongs to: a fresh UUID starts one, reusing a UUID continues it. The chat + supplies it itself; a run driven any other way has to pass it or the server refuses the job ### Tool Naming Rules diff --git a/system_prompts/auto-generated/prompts.ts b/system_prompts/auto-generated/prompts.ts index 08ad8ef6c1..9485e409a8 100644 --- a/system_prompts/auto-generated/prompts.ts +++ b/system_prompts/auto-generated/prompts.ts @@ -133,9 +133,60 @@ tool, \`websearch\` for web search. } \`\`\` -- \`provider\` is a static object, not a bare resource string: \`{ "kind": , +- \`provider\` is an object, not a bare resource string: \`{ "kind": , "resource": "$res:", "model": }\`. Required unless the module links to a saved - agent through \`value.agent\` + agent through \`value.agent\`. Static is right for a flow run from a form; a chat flow wires its + fields to flow inputs instead — see below + +### Chat-Mode Flows + +A flow with \`value.chat_input_enabled: true\` is run from a chat instead of a form: the composer +sends one message per turn and renders the conversation. It needs a required \`user_message\` string +input, read by the agent. Any other flow input the composer does not edit itself is asked for +under Configure inputs. + +**A static \`provider\` gives a chat that cannot change its model.** Feed it from flow inputs +instead, either way round: one input carrying the whole object (\`"expr": "flow_input.model_config"\`) +makes every field editable, or wire it field by field to fix some and expose others. A field the +chat can write becomes a control in the composer — a provider picker, a model list, a thinking +control — and a field left static is fixed, with no control drawn for it. \`kind\` is the one +exception: the composer writes it only together with \`resource\`, since a provider is picked as a +pair, so a \`kind\` input wired on its own stays askable under Configure inputs and nothing the run +needs becomes unreachable. + +\`\`\`json +{ + "id": "chat_agent", + "value": { + "type": "aiagent", + "input_transforms": { + "provider": { + "type": "javascript", + "expr": "({ kind: 'anthropic', resource: '$res:f/ai/claude', model: flow_input.model, reasoning_effort: flow_input.thinking })" + }, + "user_message": { "type": "javascript", "expr": "flow_input.user_message" }, + "user_attachments": { "type": "javascript", "expr": "flow_input.files" }, + "memory": { "type": "static", "value": { "kind": "auto", "context_length": 10 } }, + "streaming": { "type": "static", "value": true }, + "output_type": { "type": "static", "value": "text" } + }, + "tools": [] + } +} +\`\`\` + +- Wiring field by field means one object literal whose values are literals or bare \`flow_input.x\` + references. A spread, a call or a computed key leaves the composer unable to tell which input + feeds which field, so it offers no control at all — a bare \`flow_input.x\` for the whole object + is read instead as that one input carrying every field +- \`memory\` is what lets the agent see earlier turns; without it every message starts from nothing +- \`streaming\` on makes the answer and its thinking appear token by token instead of all at once +- \`user_attachments\` points at a flow input typed as an array of s3 objects + (\`{ "type": "array", "items": { "type": "object", "resourceType": "s3object" } }\`), so files + sent with a message reach the agent +- Running one needs a \`memory_id\` **query parameter** — not a flow argument — naming the + conversation the turn belongs to: a fresh UUID starts one, reusing a UUID continues it. The chat + supplies it itself; a run driven any other way has to pass it or the server refuses the job ### Tool Naming Rules diff --git a/system_prompts/auto-generated/skills/write-flow/SKILL.md b/system_prompts/auto-generated/skills/write-flow/SKILL.md index 3556cbfff0..da611bf168 100644 --- a/system_prompts/auto-generated/skills/write-flow/SKILL.md +++ b/system_prompts/auto-generated/skills/write-flow/SKILL.md @@ -190,9 +190,60 @@ tool, `websearch` for web search. } ``` -- `provider` is a static object, not a bare resource string: `{ "kind": , +- `provider` is an object, not a bare resource string: `{ "kind": , "resource": "$res:", "model": }`. Required unless the module links to a saved - agent through `value.agent` + agent through `value.agent`. Static is right for a flow run from a form; a chat flow wires its + fields to flow inputs instead — see below + +### Chat-Mode Flows + +A flow with `value.chat_input_enabled: true` is run from a chat instead of a form: the composer +sends one message per turn and renders the conversation. It needs a required `user_message` string +input, read by the agent. Any other flow input the composer does not edit itself is asked for +under Configure inputs. + +**A static `provider` gives a chat that cannot change its model.** Feed it from flow inputs +instead, either way round: one input carrying the whole object (`"expr": "flow_input.model_config"`) +makes every field editable, or wire it field by field to fix some and expose others. A field the +chat can write becomes a control in the composer — a provider picker, a model list, a thinking +control — and a field left static is fixed, with no control drawn for it. `kind` is the one +exception: the composer writes it only together with `resource`, since a provider is picked as a +pair, so a `kind` input wired on its own stays askable under Configure inputs and nothing the run +needs becomes unreachable. + +```json +{ + "id": "chat_agent", + "value": { + "type": "aiagent", + "input_transforms": { + "provider": { + "type": "javascript", + "expr": "({ kind: 'anthropic', resource: '$res:f/ai/claude', model: flow_input.model, reasoning_effort: flow_input.thinking })" + }, + "user_message": { "type": "javascript", "expr": "flow_input.user_message" }, + "user_attachments": { "type": "javascript", "expr": "flow_input.files" }, + "memory": { "type": "static", "value": { "kind": "auto", "context_length": 10 } }, + "streaming": { "type": "static", "value": true }, + "output_type": { "type": "static", "value": "text" } + }, + "tools": [] + } +} +``` + +- Wiring field by field means one object literal whose values are literals or bare `flow_input.x` + references. A spread, a call or a computed key leaves the composer unable to tell which input + feeds which field, so it offers no control at all — a bare `flow_input.x` for the whole object + is read instead as that one input carrying every field +- `memory` is what lets the agent see earlier turns; without it every message starts from nothing +- `streaming` on makes the answer and its thinking appear token by token instead of all at once +- `user_attachments` points at a flow input typed as an array of s3 objects + (`{ "type": "array", "items": { "type": "object", "resourceType": "s3object" } }`), so files + sent with a message reach the agent +- Running one needs a `memory_id` **query parameter** — not a flow argument — naming the + conversation the turn belongs to: a fresh UUID starts one, reusing a UUID continues it. The chat + supplies it itself; a run driven any other way has to pass it or the server refuses the job ### Tool Naming Rules diff --git a/system_prompts/base/flow-base.md b/system_prompts/base/flow-base.md index e70ac3d42a..5e922496bd 100644 --- a/system_prompts/base/flow-base.md +++ b/system_prompts/base/flow-base.md @@ -102,9 +102,60 @@ tool, `websearch` for web search. } ``` -- `provider` is a static object, not a bare resource string: `{ "kind": , +- `provider` is an object, not a bare resource string: `{ "kind": , "resource": "$res:", "model": }`. Required unless the module links to a saved - agent through `value.agent` + agent through `value.agent`. Static is right for a flow run from a form; a chat flow wires its + fields to flow inputs instead — see below + +### Chat-Mode Flows + +A flow with `value.chat_input_enabled: true` is run from a chat instead of a form: the composer +sends one message per turn and renders the conversation. It needs a required `user_message` string +input, read by the agent. Any other flow input the composer does not edit itself is asked for +under Configure inputs. + +**A static `provider` gives a chat that cannot change its model.** Feed it from flow inputs +instead, either way round: one input carrying the whole object (`"expr": "flow_input.model_config"`) +makes every field editable, or wire it field by field to fix some and expose others. A field the +chat can write becomes a control in the composer — a provider picker, a model list, a thinking +control — and a field left static is fixed, with no control drawn for it. `kind` is the one +exception: the composer writes it only together with `resource`, since a provider is picked as a +pair, so a `kind` input wired on its own stays askable under Configure inputs and nothing the run +needs becomes unreachable. + +```json +{ + "id": "chat_agent", + "value": { + "type": "aiagent", + "input_transforms": { + "provider": { + "type": "javascript", + "expr": "({ kind: 'anthropic', resource: '$res:f/ai/claude', model: flow_input.model, reasoning_effort: flow_input.thinking })" + }, + "user_message": { "type": "javascript", "expr": "flow_input.user_message" }, + "user_attachments": { "type": "javascript", "expr": "flow_input.files" }, + "memory": { "type": "static", "value": { "kind": "auto", "context_length": 10 } }, + "streaming": { "type": "static", "value": true }, + "output_type": { "type": "static", "value": "text" } + }, + "tools": [] + } +} +``` + +- Wiring field by field means one object literal whose values are literals or bare `flow_input.x` + references. A spread, a call or a computed key leaves the composer unable to tell which input + feeds which field, so it offers no control at all — a bare `flow_input.x` for the whole object + is read instead as that one input carrying every field +- `memory` is what lets the agent see earlier turns; without it every message starts from nothing +- `streaming` on makes the answer and its thinking appear token by token instead of all at once +- `user_attachments` points at a flow input typed as an array of s3 objects + (`{ "type": "array", "items": { "type": "object", "resourceType": "s3object" } }`), so files + sent with a message reach the agent +- Running one needs a `memory_id` **query parameter** — not a flow argument — naming the + conversation the turn belongs to: a fresh UUID starts one, reusing a UUID continues it. The chat + supplies it itself; a run driven any other way has to pass it or the server refuses the job ### Tool Naming Rules