From b100606da6a61f2dbcb24516363f43643bc917e3 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 3 Sep 2026 17:02:08 +0000 Subject: [PATCH 001/130] fix: patch critical CVEs in the worker image (#10962) * fix: patch critical CVEs in the worker image (go, node, php, helm, libtiff) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01TT3CyQED8iwwmsKMttk6PP * ci: run the backend tests on node 24 to match the image Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01TT3CyQED8iwwmsKMttk6PP --------- Co-authored-by: Claude Fable 5.1 --- .github/workflows/backend-test.yml | 2 +- Dockerfile | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/backend-test.yml b/.github/workflows/backend-test.yml index cdd0e2a212..9f65c2ae47 100644 --- a/.github/workflows/backend-test.yml +++ b/.github/workflows/backend-test.yml @@ -61,7 +61,7 @@ jobs: bun-version: 1.4.0 - uses: actions/setup-node@v4 with: - node-version: "20" + node-version: "24" - uses: astral-sh/setup-uv@v6.2.1 with: version: "0.11.24" diff --git a/Dockerfile b/Dockerfile index 8ecee96725..6a290c3553 100644 --- a/Dockerfile +++ b/Dockerfile @@ -141,9 +141,9 @@ FROM ${DEBIAN_IMAGE} ARG TARGETPLATFORM ARG POWERSHELL_VERSION=7.5.0 ARG KUBECTL_VERSION=1.36.2 -ARG HELM_VERSION=3.21.2 +ARG HELM_VERSION=3.21.4 # NOTE: If changing, also change go version in workspace dependencies template at WorkspaceDependenciesEditor.svelte -ARG GO_VERSION=1.26.0 +ARG GO_VERSION=1.26.8 ARG APP=/usr/src/app ARG WITH_POWERSHELL=true ARG WITH_KUBECTL=true @@ -250,8 +250,8 @@ RUN UV_CACHE_DIR=/tmp/build_cache/uv UV_PYTHON_INSTALL_DIR=/tmp/build_cache/py_r RUN UV_CACHE_DIR=/tmp/build_cache/uv UV_PYTHON_INSTALL_DIR=/tmp/build_cache/py_runtime uv python install $LATEST_STABLE_PY --compile-bytecode -RUN curl -sL https://deb.nodesource.com/setup_20.x | bash - -RUN apt-get -y update && apt-get install -y curl procps nodejs awscli && apt-get clean \ +RUN curl -sL https://deb.nodesource.com/setup_24.x | bash - +RUN apt-get -y update && apt-get install -y --no-install-recommends curl procps nodejs awscli && apt-get clean \ && rm -rf /var/lib/apt/lists/* # go build is slower the first time it is ran, so we prewarm it in the build @@ -299,7 +299,7 @@ RUN bun install -g windmill-cli \ RUN curl -fsSL https://claude.ai/install.sh | bash \ && cp /root/.local/share/claude/versions/* /usr/bin/claude -COPY --from=php:8.3.30-cli-trixie /usr/local/bin/php /usr/bin/php +COPY --from=php:8.3.33-cli-trixie /usr/local/bin/php /usr/bin/php COPY --from=composer:2.9.5 /usr/bin/composer /usr/bin/composer # add the docker client to call docker from a worker if enabled From 79426a1a68a6b19e12af4633b8a79d07a103a106 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 3 Sep 2026 20:23:43 +0000 Subject: [PATCH 002/130] feat: reconcile IdP instance groups from the SSO groups claim (#10957) * feat: add sso_groups_claim setting for login-time instance group sync Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01YESxWqzt959S6TY6vbc4eG * chore: bump ee-repo-ref for the SSO groups claim reconcile Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01YESxWqzt959S6TY6vbc4eG * chore: update ee-repo-ref to 3b89bfc11314a326a191101cfe3ef65f6f7f82a8 This commit updates the EE repository reference after PR #774 was merged in windmill-ee-private. Previous ee-repo-ref: e388527f9adbbe466fe050ca8d1d236ce3342bc3 New ee-repo-ref: 3b89bfc11314a326a191101cfe3ef65f6f7f82a8 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Fable 5.1 Co-authored-by: windmill-internal-app[bot] --- backend/ee-repo-ref.txt | 2 +- backend/windmill-api/openapi.yaml | 2 ++ backend/windmill-common/src/global_settings.rs | 3 +++ docs/feature-telemetry.md | 4 ++-- frontend/src/lib/components/InstanceSettings.svelte | 10 ++++++---- .../lib/components/auditLogs/AuditLogsFilters.svelte | 2 ++ frontend/src/lib/components/instanceSettings.ts | 10 ++++++++++ 7 files changed, 26 insertions(+), 7 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 9f37cbedb0..354e88bef1 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -f5b783d2f7608e1ff3a817caa8b719e06f8b8981 +3b89bfc11314a326a191101cfe3ef65f6f7f82a8 diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index f2bd3a543c..a27be7d295 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -28994,6 +28994,8 @@ components: - "igroup.delete" - "igroup.adduser" - "igroup.removeuser" + - "instance_groups.jit_adduser" + - "instance_groups.jit_removeuser" - "variables.decrypt_secret" - "workspaces.read_encryption_key" - "workspaces.edit_command_script" diff --git a/backend/windmill-common/src/global_settings.rs b/backend/windmill-common/src/global_settings.rs index 5c4b4bddab..d38396562b 100644 --- a/backend/windmill-common/src/global_settings.rs +++ b/backend/windmill-common/src/global_settings.rs @@ -94,6 +94,9 @@ pub const AUTOMATE_USERNAME_CREATION_SETTING: &str = "automate_username_creation pub const DISABLE_WORKSPACE_INVITE_EMAILS_SETTING: &str = "disable_workspace_invite_emails"; pub const DISABLE_PASSWORD_LOGIN_SETTING: &str = "disable_password_login"; pub const AUTO_LOGIN_PROVIDER_SETTING: &str = "auto_login_provider"; +/// Name of the SAML attribute or OIDC userinfo claim carrying the user's IdP groups. Unset or +/// empty leaves instance-group membership entirely to SCIM. +pub const SSO_GROUPS_CLAIM_SETTING: &str = "sso_groups_claim"; pub const HUB_BASE_URL_SETTING: &str = "hub_base_url"; pub const HUB_ACCESSIBLE_URL_SETTING: &str = "hub_accessible_url"; pub const DISABLE_HUB_SETTING: &str = "disable_hub"; diff --git a/docs/feature-telemetry.md b/docs/feature-telemetry.md index 5d7f0b2c29..5cef3c9364 100644 --- a/docs/feature-telemetry.md +++ b/docs/feature-telemetry.md @@ -4,9 +4,9 @@ anonymous usage-stats payload. It answers "does anyone use this, and which variant do they pick" without any identifying data leaving the instance. -It currently carries 28 registered actions across fourteen features (`ai_session`, `ai_chat`, +It currently carries 32 registered actions across fifteen features (`ai_session`, `ai_chat`, `ai_fix`, `ai_agent`, `ai_agent_eval`, `flow_editor`, `flow_run`, `flow_step`, `run_form`, -`debugger`, `trigger`, `command_script`, `hub_script`, `usage_meter`). Nearly all of the +`debugger`, `trigger`, `command_script`, `hub_script`, `usage_meter`, `sso_groups_claim`). Nearly all of the product is uninstrumented, so new user-facing work is the opportunity to change that. ## When to instrument diff --git a/frontend/src/lib/components/InstanceSettings.svelte b/frontend/src/lib/components/InstanceSettings.svelte index ce5323fe2b..b321e76693 100644 --- a/frontend/src/lib/components/InstanceSettings.svelte +++ b/frontend/src/lib/components/InstanceSettings.svelte @@ -1071,8 +1071,9 @@ >feature usage (counts of which product features are used, including AI provider and model identifiers, the names of public hub scripts used, the languages debug sessions are started for, whether AI chat skills are turned on or off and how often one is - loaded, and the plan tier and quota shown when the execution meter is opened, last 30 - days)
  • feature adoption (counts of which flow, script, trigger and worker features your @@ -1125,8 +1126,9 @@ >feature usage (counts of which product features are used, including AI provider and model identifiers, the names of public hub scripts used, the languages debug sessions are started for, whether AI chat skills are turned on or off and how often one is - loaded, and the plan tier and quota shown when the execution meter is opened, last 30 - days)
  • feature adoption (counts of which flow, script, trigger and worker features your diff --git a/frontend/src/lib/components/auditLogs/AuditLogsFilters.svelte b/frontend/src/lib/components/auditLogs/AuditLogsFilters.svelte index 213beda001..96d341dc72 100644 --- a/frontend/src/lib/components/auditLogs/AuditLogsFilters.svelte +++ b/frontend/src/lib/components/auditLogs/AuditLogsFilters.svelte @@ -216,6 +216,8 @@ INSTANCE_GROUPS_SCIM_CREATE: 'instance_groups.scim_create', INSTANCE_GROUPS_SCIM_DELETE: 'instance_groups.scim_delete', INSTANCE_GROUPS_SCIM_UPDATE: 'instance_groups.scim_update', + INSTANCE_GROUPS_JIT_ADDUSER: 'instance_groups.jit_adduser', + INSTANCE_GROUPS_JIT_REMOVEUSER: 'instance_groups.jit_removeuser', VARIABLES_DECRYPT_SECRET: 'variables.decrypt_secret', WORKSPACES_READ_ENCRYPTION_KEY: 'workspaces.read_encryption_key', WORKSPACES_EDIT_COMMAND_SCRIPT: 'workspaces.edit_command_script', diff --git a/frontend/src/lib/components/instanceSettings.ts b/frontend/src/lib/components/instanceSettings.ts index eb8c4c86c4..8bbca5b4be 100644 --- a/frontend/src/lib/components/instanceSettings.ts +++ b/frontend/src/lib/components/instanceSettings.ts @@ -662,6 +662,16 @@ export const settings: Record = { fieldType: 'text', placeholder: 'okta', storage: 'setting' + }, + { + label: 'SSO groups claim', + description: + 'Name of the SAML attribute or OIDC userinfo claim carrying the user\'s IdP groups ("http://schemas.microsoft.com/ws/2008/06/identity/claims/groups" on Entra SAML, "groups" for most OIDC providers). Its values must be the same group ids that SCIM stored as the instance groups\' external id (Entra emits object ids in both), since matching is by external id only. When set, every SSO login reconciles the user\'s membership in those SCIM-provisioned instance groups against the claim, so IdP group changes take effect at the next login instead of waiting for the SCIM push. Instance groups without an external id are never touched, and a login whose claim is absent or empty changes nothing. Leave empty to disable.', + key: 'sso_groups_claim', + fieldType: 'text', + placeholder: 'groups', + storage: 'setting', + ee_only: '' } ], 'DB Health': [], From 3e3d2a636334146014926841949372083e6e8516 Mon Sep 17 00:00:00 2001 From: Diego Imbert <70353967+diegoimbert@users.noreply.github.com> Date: Thu, 3 Sep 2026 22:30:19 +0200 Subject: [PATCH 003/130] fix: keep braces inside string tool arguments out of JSON depth count (#10965) Claude-Session: https://claude.ai/code/session_013vvU4UWCpib25ovmmAD7HH Co-authored-by: Claude Opus 5 (1M context) --- .../components/copilot/lib.toolCalls.test.ts | 31 +++++++++++++++++++ frontend/src/lib/components/copilot/lib.ts | 16 ++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/frontend/src/lib/components/copilot/lib.toolCalls.test.ts b/frontend/src/lib/components/copilot/lib.toolCalls.test.ts index ef2708e570..c665cc964e 100644 --- a/frontend/src/lib/components/copilot/lib.toolCalls.test.ts +++ b/frontend/src/lib/components/copilot/lib.toolCalls.test.ts @@ -134,6 +134,37 @@ describe('parseOpenAICompletion tool call arguments', () => { expect(addedMessages).toEqual(messages) }) + it('keeps braces inside string arguments out of the object-depth count', async () => { + const { parseOpenAICompletion } = await import('./lib') + const fn = vi.fn().mockResolvedValue('tool ok') + const messages: ChatCompletionMessageParam[] = [] + + await parseOpenAICompletion( + streamOf([ + toolCallChunk({ + id: 'call_1', + function: { name: 'patch_app_file', arguments: '{"new_string": ' } + }), + toolCallChunk({ function: { arguments: '"hello } goodbye \\" } "}' } }) + ]), + createCallbacks(), + messages, + [], + [createTool(fn)] as any, + {}, + undefined, + { workspace: 'test' } + ) + + expect(fn).toHaveBeenCalledWith( + expect.objectContaining({ args: { new_string: 'hello } goodbye " } ' } }) + ) + const assistant = messages.find((m) => m.role === 'assistant') as any + expect(assistant.tool_calls[0].function.arguments).toBe( + '{"new_string": "hello } goodbye \\" } "}' + ) + }) + it('marks only the executing tool call as loading when one message has several', async () => { const { parseOpenAICompletion } = await import('./lib') const statuses: Record = {} diff --git a/frontend/src/lib/components/copilot/lib.ts b/frontend/src/lib/components/copilot/lib.ts index fc8a00727f..19f350a665 100644 --- a/frontend/src/lib/components/copilot/lib.ts +++ b/frontend/src/lib/components/copilot/lib.ts @@ -1177,9 +1177,21 @@ export async function getCompletion( function extractFirstJSON(str: string) { let depth = 0, i = 0 + // Braces inside string values are not depth changes, so the scan tracks + // quoting and escaping: otherwise an argument such as {"a": "} "} is cut short. + let inString = false, + escaped = false for (; i < str.length; i++) { - if (str[i] === '{') depth++ - else if (str[i] === '}' && --depth === 0) break + const ch = str[i] + if (inString) { + if (escaped) escaped = false + else if (ch === '\\') escaped = true + else if (ch === '"') inString = false + continue + } + if (ch === '"') inString = true + else if (ch === '{') depth++ + else if (ch === '}' && --depth === 0) break } return str.slice(0, i + 1) } From 6a7a6d9144fce55ef6850a1f63274bba548b2e8b Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 3 Sep 2026 22:40:34 +0000 Subject: [PATCH 004/130] chore: stop denying reads of secret files in claude settings (#10968) Any Read() deny rule makes Claude Code resolve the file operands of every Bash command that reads files. A path it cannot resolve, such as one that follows a cd into a directory the analyzer does not track, escalates to a permission prompt even under bypassPermissions. A plain recursive grep in the repo root escalates too, because it could reach .env. Drop the read rules and widen the write rules to cover the same files, so secrets still cannot be written through Edit, Write, or a shell redirect. Reads of those files are no longer blocked. Claude-Session: https://claude.ai/code/session_01RNCupPk2yewQT1JMNjkV8M Co-authored-by: Claude Opus 5 (1M context) --- .claude/settings.json | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index a464ca3719..37aaeeb0f5 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -55,22 +55,18 @@ "mcp__claude_ai_Gmail__list_drafts" ], "deny": [ - "Read(.env)", - "Read(.env.*)", - "Read(**/.env)", - "Read(**/.env.*)", - "Read(**/secrets/**)", - "Read(**/*.pem)", - "Read(**/*.key)", - "Read(**/credentials.json)", - "Read(**/.secret*)", - "Read(**/.secrets*)", - "Read(**/*.secret)", - "Read(**/*.secrets)", "Edit(.env)", "Edit(.env.*)", "Edit(**/.env)", - "Edit(**/.env.*)" + "Edit(**/.env.*)", + "Edit(**/secrets/**)", + "Edit(**/*.pem)", + "Edit(**/*.key)", + "Edit(**/credentials.json)", + "Edit(**/.secret*)", + "Edit(**/.secrets*)", + "Edit(**/*.secret)", + "Edit(**/*.secrets)" ], "ask": [ "Bash(rmdir:*)", From 11138284acc4c1d8673e86823c7f74c9e1f419e6 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 3 Sep 2026 23:01:36 +0000 Subject: [PATCH 005/130] fix: deploy a relocked script version only when its lock changed (#10966) * fix: deploy a relocked script version only when its lock changed Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GdEb6gzCZ2qXmAQJAeMf9W * fix: write the unchanged relock hash under the row lock and skip the phantom tally Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GdEb6gzCZ2qXmAQJAeMf9W * fix: requeue a superseded relock and read the live head past the script cache Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GdEb6gzCZ2qXmAQJAeMf9W * fix: re-read the relock head after waiting on its lock and keep module locks Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GdEb6gzCZ2qXmAQJAeMf9W * fix: bound the relock head re-read instead of reading once Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GdEb6gzCZ2qXmAQJAeMf9W * chore: refresh the sqlx cache entry for the re-indented lock write Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GdEb6gzCZ2qXmAQJAeMf9W * test: pin the waiting-relock requeue and the multi-file importer no-op Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GdEb6gzCZ2qXmAQJAeMf9W --------- Co-authored-by: Claude Fable 5.1 --- ...3b9c0e6e371136b565f9b0f5a3393ef8d8d57.json | 19 - ...42ae6d6d4b6731c3f62e7b85ae7a4f1b666a7.json | 16 - ...b7a545959c209c550e48bf108734f293e18e4.json | 19 + ...8031783fa7fbbbf86a291b6f8f8f875d45637.json | 19 + backend/tests/relock_noop.rs | 415 ++++++++++++++++++ backend/tests/relock_skip.rs | 7 +- backend/windmill-common/src/scripts.rs | 72 +-- backend/windmill-dep-map/src/lib.rs | 58 ++- backend/windmill-queue/src/jobs.rs | 31 +- backend/windmill-types/src/scripts.rs | 2 +- .../windmill-worker/src/worker_lockfiles.rs | 384 +++++++++++++--- 11 files changed, 860 insertions(+), 182 deletions(-) delete mode 100644 backend/.sqlx/query-49b18e987e2dfa3c7ab915757ff3b9c0e6e371136b565f9b0f5a3393ef8d8d57.json delete mode 100644 backend/.sqlx/query-96aa1db2859c80d37ffecc2885442ae6d6d4b6731c3f62e7b85ae7a4f1b666a7.json create mode 100644 backend/.sqlx/query-a0ec5048ddb7640b4407013ed45b7a545959c209c550e48bf108734f293e18e4.json create mode 100644 backend/.sqlx/query-ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637.json create mode 100644 backend/tests/relock_noop.rs diff --git a/backend/.sqlx/query-49b18e987e2dfa3c7ab915757ff3b9c0e6e371136b565f9b0f5a3393ef8d8d57.json b/backend/.sqlx/query-49b18e987e2dfa3c7ab915757ff3b9c0e6e371136b565f9b0f5a3393ef8d8d57.json deleted file mode 100644 index 8d86ff3db6..0000000000 --- a/backend/.sqlx/query-49b18e987e2dfa3c7ab915757ff3b9c0e6e371136b565f9b0f5a3393ef8d8d57.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "WITH update_lock AS (\n UPDATE script SET lock = $1, modules = COALESCE($6, modules) WHERE hash = $2 AND workspace_id = $3\n )\n INSERT INTO lock_hash (workspace_id, path, lockfile_hash)\n VALUES ($3, $4, $5)\n ON CONFLICT (workspace_id, path) DO UPDATE SET lockfile_hash = $5", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Text", - "Int8", - "Text", - "Varchar", - "Int8", - "Jsonb" - ] - }, - "nullable": [] - }, - "hash": "49b18e987e2dfa3c7ab915757ff3b9c0e6e371136b565f9b0f5a3393ef8d8d57" -} diff --git a/backend/.sqlx/query-96aa1db2859c80d37ffecc2885442ae6d6d4b6731c3f62e7b85ae7a4f1b666a7.json b/backend/.sqlx/query-96aa1db2859c80d37ffecc2885442ae6d6d4b6731c3f62e7b85ae7a4f1b666a7.json deleted file mode 100644 index 733478e0e9..0000000000 --- a/backend/.sqlx/query-96aa1db2859c80d37ffecc2885442ae6d6d4b6731c3f62e7b85ae7a4f1b666a7.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n INSERT INTO script\n (workspace_id, hash, path, parent_hashes, summary, description, content, created_by, schema, is_template, extra_perms, lock, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels)\n\n SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, content, created_by, schema, is_template, extra_perms, NULL, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels\n\n FROM script WHERE hash = $2 AND workspace_id = $3;\n ", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Int8", - "Int8", - "Text" - ] - }, - "nullable": [] - }, - "hash": "96aa1db2859c80d37ffecc2885442ae6d6d4b6731c3f62e7b85ae7a4f1b666a7" -} diff --git a/backend/.sqlx/query-a0ec5048ddb7640b4407013ed45b7a545959c209c550e48bf108734f293e18e4.json b/backend/.sqlx/query-a0ec5048ddb7640b4407013ed45b7a545959c209c550e48bf108734f293e18e4.json new file mode 100644 index 0000000000..f4c4fd31de --- /dev/null +++ b/backend/.sqlx/query-a0ec5048ddb7640b4407013ed45b7a545959c209c550e48bf108734f293e18e4.json @@ -0,0 +1,19 @@ +{ + "db_name": "PostgreSQL", + "query": "WITH update_lock AS (\n UPDATE script SET lock = $1, modules = COALESCE($6, modules) WHERE hash = $2 AND workspace_id = $3\n )\n INSERT INTO lock_hash (workspace_id, path, lockfile_hash)\n VALUES ($3, $4, $5)\n ON CONFLICT (workspace_id, path) DO UPDATE SET lockfile_hash = $5", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Int8", + "Text", + "Varchar", + "Int8", + "Jsonb" + ] + }, + "nullable": [] + }, + "hash": "a0ec5048ddb7640b4407013ed45b7a545959c209c550e48bf108734f293e18e4" +} diff --git a/backend/.sqlx/query-ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637.json b/backend/.sqlx/query-ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637.json new file mode 100644 index 0000000000..bea6be2a7f --- /dev/null +++ b/backend/.sqlx/query-ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637.json @@ -0,0 +1,19 @@ +{ + "db_name": "PostgreSQL", + "query": "\n INSERT INTO script\n (workspace_id, hash, path, parent_hashes, summary, description, content, created_by, schema, is_template, extra_perms, lock, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels, lock_error_logs, created_at)\n\n SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, content, created_by, schema, is_template, extra_perms, $4::text, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, COALESCE($5::jsonb, modules), labels, $6::text, clock_timestamp()\n\n FROM script WHERE hash = $2 AND workspace_id = $3;\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8", + "Int8", + "Text", + "Text", + "Jsonb", + "Text" + ] + }, + "nullable": [] + }, + "hash": "ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637" +} diff --git a/backend/tests/relock_noop.rs b/backend/tests/relock_noop.rs new file mode 100644 index 0000000000..5cb8e3f51e --- /dev/null +++ b/backend/tests/relock_noop.rs @@ -0,0 +1,415 @@ +use sqlx::{Pool, Postgres}; +use tokio_stream::StreamExt; +use windmill_api_client::types::NewScript; +use windmill_common::scripts::{deploy_relocked_version, fetch_script_for_update}; +use windmill_test_utils::*; + +const W: &str = "test-workspace"; + +const A: &str = r#"export async function main() { return "a" }"#; +const A_COMMENTED: &str = r#"// same dependencies, different content +export async function main() { return "a" }"#; +const A_WITH_LODASH: &str = r#"import _ from "lodash@4.17.21"; +export async function main() { return _.trim(" a ") }"#; +const B: &str = r#"import { main as a } from "/f/rel/a.ts"; +export async function main() { return "b" + (await a()) }"#; +const C: &str = r#"import { main as b } from "/f/rel/b.ts"; +export async function main() { return "c" + (await b()) }"#; + +fn bun_script(path: &str, content: &str, parent_hash: Option) -> NewScript { + NewScript { + draft_only: None, + content: content.into(), + language: windmill_api_client::types::ScriptLang::Bun, + lock: None, + parent_hash, + path: path.into(), + concurrent_limit: None, + concurrency_time_window_s: None, + cache_ttl: None, + dedicated_worker: None, + description: "".to_string(), + envs: vec![], + is_template: None, + kind: None, + summary: "".to_string(), + tag: None, + schema: std::collections::HashMap::new(), + ws_error_handler_muted: Some(false), + priority: None, + delete_after_secs: None, + timeout: None, + restart_unless_cancelled: None, + deployment_message: None, + concurrency_key: None, + visible_to_runner_only: None, + auto_kind: None, + codebase: None, + has_preprocessor: None, + on_behalf_of_email: None, + assets: vec![], + modules: None, + } +} + +#[derive(sqlx::FromRow, Debug)] +struct Version { + hash: i64, + archived: bool, + lock: Option, + created_at: chrono::DateTime, +} + +/// Every version of `path`, oldest first. +async fn versions(db: &Pool, path: &str) -> Vec { + sqlx::query_as( + "SELECT hash, archived, lock, created_at FROM script + WHERE workspace_id = $1 AND path = $2 ORDER BY created_at", + ) + .bind(W) + .bind(path) + .fetch_all(db) + .await + .unwrap() +} + +fn live(versions: &[Version]) -> &Version { + versions.iter().rev().find(|v| !v.archived).unwrap() +} + +/// `(path, status, logs)` of every dependency job created after `since`, in completion order. +async fn dependency_jobs_since( + db: &Pool, + since: chrono::DateTime, +) -> Vec<(String, String, String)> { + sqlx::query_as( + "SELECT j.runnable_path, c.status::text, COALESCE(l.logs, '') FROM v2_job_completed c + JOIN v2_job j ON j.id = c.id + LEFT JOIN job_logs l ON l.job_id = c.id + WHERE j.kind = 'dependencies' AND j.created_at > $1 + ORDER BY c.started_at", + ) + .bind(since) + .fetch_all(db) + .await + .unwrap() +} + +async fn wait_for_jobs( + completed: &mut (impl futures::Stream + Unpin), + count: usize, +) { + for _ in 0..count { + completed.next().await; + } + // Then let anything else that was queued run out, so a job the assertions say must not + // exist would have shown up here. + while let Ok(Some(_)) = + tokio::time::timeout(std::time::Duration::from_secs(2), completed.next()).await + {} +} + +/// A redeploy of an imported script whose dependencies did not move relocks its importer, +/// and that relock must deploy nothing: no new version, and no dependency job for the +/// importer's own importers. A redeploy that does change the dependencies still walks the +/// whole chain with a new version at each step. +#[sqlx::test(fixtures("base"))] +async fn relative_import_relock_deploys_only_when_the_lock_changed( + db: Pool, +) -> anyhow::Result<()> { + std::env::set_var("DEPENDENCY_JOB_DEBOUNCE_DELAY", "0"); + let (client, port, _s) = init_client(db.clone()).await; + let mut completed = listen_for_completed_jobs(&db).await; + + in_test_worker( + &db, + async { + // One at a time: each deploy's dependency job records the importer's edges, and an + // importer whose edges are recorded is what a later relock of it can skip on. + for (path, content) in [("f/rel/a", A), ("f/rel/b", B), ("f/rel/c", C)] { + client + .create_script(W, &bun_script(path, content, None)) + .await + .unwrap(); + wait_for_jobs(&mut completed, 1).await; + } + let b_before = versions(&db, "f/rel/b").await; + let c_before = versions(&db, "f/rel/c").await; + assert_eq!(b_before.len(), 1); + assert_eq!(c_before.len(), 1); + + // Content-only change on the leaf. + let since = chrono::Utc::now(); + let a_hash = live(&versions(&db, "f/rel/a").await).hash; + client + .create_script( + W, + &bun_script("f/rel/a", A_COMMENTED, Some(format!("{a_hash:016x}"))), + ) + .await + .unwrap(); + wait_for_jobs(&mut completed, 2).await; + + let jobs = dependency_jobs_since(&db, since).await; + let paths: Vec<&str> = jobs.iter().map(|(p, _, _)| p.as_str()).collect(); + assert_eq!( + paths, + ["f/rel/a", "f/rel/b"], + "the leaf's own job and one no-op relock of its importer, and nothing for c" + ); + assert!( + jobs[1] + .2 + .contains("Lock unchanged: no new version deployed"), + "b's relock should have found its lock unchanged: {}", + jobs[1].2 + ); + let b_after = versions(&db, "f/rel/b").await; + let c_after = versions(&db, "f/rel/c").await; + assert_eq!( + b_after.len(), + 1, + "an unchanged relock must not mint a version" + ); + assert_eq!(live(&b_after).hash, live(&b_before).hash); + assert_eq!(c_after.len(), 1); + assert_eq!(live(&c_after).hash, live(&c_before).hash); + + // A dependency change on the leaf. + let since = chrono::Utc::now(); + let a_hash = live(&versions(&db, "f/rel/a").await).hash; + client + .create_script( + W, + &bun_script("f/rel/a", A_WITH_LODASH, Some(format!("{a_hash:016x}"))), + ) + .await + .unwrap(); + wait_for_jobs(&mut completed, 3).await; + + let jobs = dependency_jobs_since(&db, since).await; + let paths: Vec<&str> = jobs.iter().map(|(p, _, _)| p.as_str()).collect(); + assert_eq!(paths, ["f/rel/a", "f/rel/b", "f/rel/c"]); + for path in ["f/rel/b", "f/rel/c"] { + let vs = versions(&db, path).await; + assert_eq!( + vs.len(), + 2, + "{path}: a changed relock deploys a new version" + ); + assert!( + vs[0].archived && !vs[1].archived, + "{path}: parent archived, child live" + ); + assert!(vs[0].created_at < vs[1].created_at, "{path}: lineage order"); + assert!( + vs[1].lock.as_deref().unwrap_or("").contains("lodash"), + "{path}: the new version carries the new lock: {:?}", + vs[1].lock + ); + } + }, + port, + ) + .await; + + Ok(()) +} + +/// A relock that has to wait on its head's row lock, because a deploy of the same path holds +/// it, must find the version that deploy left and requeue itself for it rather than fail. The +/// blocked statement re-checks only the row it selected, which the deploy archived, and comes +/// back empty; the successor is only visible to a fresh read. +#[sqlx::test(fixtures("base"))] +async fn relock_waiting_on_a_deploy_requeues_for_its_successor( + db: Pool, +) -> anyhow::Result<()> { + std::env::set_var("DEPENDENCY_JOB_DEBOUNCE_DELAY", "0"); + let (client, port, _s) = init_client(db.clone()).await; + let mut completed = listen_for_completed_jobs(&db).await; + + in_test_worker( + &db, + async { + for (path, content) in [("f/rel/a", A), ("f/rel/b", B)] { + client + .create_script(W, &bun_script(path, content, None)) + .await + .unwrap(); + wait_for_jobs(&mut completed, 1).await; + } + + // A deploy of b that holds its head's row lock for as long as this transaction lives. + let mut deploy = db.begin().await.unwrap(); + let head = fetch_script_for_update("f/rel/b", W, &mut *deploy) + .await + .unwrap() + .unwrap(); + + let since = chrono::Utc::now(); + let a_hash = live(&versions(&db, "f/rel/a").await).hash; + client + .create_script( + W, + &bun_script("f/rel/a", A_COMMENTED, Some(format!("{a_hash:016x}"))), + ) + .await + .unwrap(); + + // b's relock skips generation and reaches its commit, where it waits on the lock. + let mut waiting = false; + for _ in 0..300 { + waiting = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM pg_stat_activity + WHERE datname = current_database() AND wait_event_type = 'Lock' + AND query LIKE '%FROM script WHERE path = $1%FOR UPDATE%')", + ) + .fetch_one(&db) + .await + .unwrap(); + if waiting { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + assert!(waiting, "b's relock never reached the row lock"); + + // The deploy lands: the head is archived and a successor with its own lock takes + // its place, while the relock is still waiting. + let lock = head.lock.clone().unwrap(); + let successor = + deploy_relocked_version(&mut deploy, head, None, Some(&lock), None, None) + .await + .unwrap(); + deploy.commit().await.unwrap(); + + // a's own job, the relock that waited, and the relock it queued for the successor. + wait_for_jobs(&mut completed, 3).await; + + let jobs = dependency_jobs_since(&db, since).await; + let paths: Vec<&str> = jobs.iter().map(|(p, _, _)| p.as_str()).collect(); + assert_eq!(paths, ["f/rel/a", "f/rel/b", "f/rel/b"], "{jobs:?}"); + assert!( + jobs.iter().all(|(_, status, _)| status == "success"), + "no relock may fail on the wait: {jobs:?}" + ); + assert!( + jobs[1] + .2 + .contains("was deployed while this lock was generated"), + "the waiting relock should have seen the successor: {}", + jobs[1].2 + ); + assert!( + jobs[2] + .2 + .contains("Lock unchanged: no new version deployed"), + "the requeued relock should find the successor's lock current: {}", + jobs[2].2 + ); + let vs = versions(&db, "f/rel/b").await; + assert_eq!( + vs.len(), + 2, + "the deploy's successor and nothing else: {vs:?}" + ); + assert_eq!(live(&vs).hash, successor); + }, + port, + ) + .await; + + Ok(()) +} + +/// A multi-file importer: on a skipped relock each module gets its own last lock back, not the +/// parent script's, so an import's content-only redeploy leaves the importer alone as well. +#[cfg(feature = "python")] +#[sqlx::test(fixtures("base"))] +async fn multi_file_importer_relock_is_a_no_op_too(db: Pool) -> anyhow::Result<()> { + std::env::set_var("DEPENDENCY_JOB_DEBOUNCE_DELAY", "0"); + let (client, port, _s) = init_client(db.clone()).await; + let mut completed = listen_for_completed_jobs(&db).await; + + let py = |path: &str, content: &str, parent_hash: Option, with_module: bool| { + let mut ns = bun_script(path, content, parent_hash); + ns.language = windmill_api_client::types::ScriptLang::Python3; + if with_module { + ns.modules = Some(std::collections::HashMap::from([( + "helper.py".to_string(), + serde_json::json!({ + "content": "def greet(x):\n return 'hi ' + x\n", + "language": "python3" + }), + )])); + } + ns + }; + async fn module_lock(db: &Pool) -> Option { + sqlx::query_scalar( + "SELECT modules->'helper.py'->>'lock' FROM script + WHERE workspace_id = $1 AND path = 'f/rel/pb' AND archived = false", + ) + .bind(W) + .fetch_one(db) + .await + .unwrap() + } + + in_test_worker( + &db, + async { + client + .create_script(W, &py("f/rel/pa", "def main():\n return 'a'\n", None, false)) + .await + .unwrap(); + wait_for_jobs(&mut completed, 1).await; + client + .create_script( + W, + &py( + "f/rel/pb", + "from f.rel.pa import main as a\nfrom .helper import greet\n\ndef main():\n return greet(a())\n", + None, + true, + ), + ) + .await + .unwrap(); + wait_for_jobs(&mut completed, 1).await; + let lock_before = module_lock(&db).await; + assert!(lock_before.is_some(), "the module got a lock of its own on deploy"); + + let since = chrono::Utc::now(); + let pa_hash = live(&versions(&db, "f/rel/pa").await).hash; + client + .create_script( + W, + &py( + "f/rel/pa", + "# same dependencies\ndef main():\n return 'a'\n", + Some(format!("{pa_hash:016x}")), + false, + ), + ) + .await + .unwrap(); + wait_for_jobs(&mut completed, 2).await; + + let jobs = dependency_jobs_since(&db, since).await; + let paths: Vec<&str> = jobs.iter().map(|(p, _, _)| p.as_str()).collect(); + assert_eq!(paths, ["f/rel/pa", "f/rel/pb"], "{jobs:?}"); + assert!( + jobs[1].2.contains("Lock unchanged: no new version deployed"), + "the multi-file importer's relock should be a no-op: {}", + jobs[1].2 + ); + assert_eq!(versions(&db, "f/rel/pb").await.len(), 1); + assert_eq!(module_lock(&db).await, lock_before, "the module keeps its own lock"); + }, + port, + ) + .await; + + Ok(()) +} diff --git a/backend/tests/relock_skip.rs b/backend/tests/relock_skip.rs index 8262a38cf4..bf5cb24cbf 100644 --- a/backend/tests/relock_skip.rs +++ b/backend/tests/relock_skip.rs @@ -266,7 +266,10 @@ def main(): .await .unwrap(); - in_test_worker(&db, wait_for_jobs_ge(&mut completed, 10), port).await; + // Empty content leaves every importer's lock as it was, so only the five direct + // importers of the default deps run a job: an unchanged script relock deploys no + // version and so queues nothing for its own importers. + in_test_worker(&db, wait_for_jobs_ge(&mut completed, 5), port).await; // Note: within a cascade, the same script may be triggered multiple times. // After the first trigger relocks and stores the hash, subsequent triggers skip. @@ -295,7 +298,7 @@ def main(): .await .unwrap(); - in_test_worker(&db, wait_for_jobs_ge(&mut completed, 10), port).await; + in_test_worker(&db, wait_for_jobs_ge(&mut completed, 5), port).await; let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await; assert!( diff --git a/backend/windmill-common/src/scripts.rs b/backend/windmill-common/src/scripts.rs index 31219621e9..d71bc50e60 100644 --- a/backend/windmill-common/src/scripts.rs +++ b/backend/windmill-common/src/scripts.rs @@ -358,31 +358,38 @@ pub async fn fetch_script_for_update<'a>( .map_err(crate::error::Error::from) } -pub struct ClonedScript { - pub old_script: NewScript, - pub new_hash: i64, -} -// TODO: What if dependency job fails, there is script with NULL in the lock -pub async fn clone_script<'c>( - path: &str, - w_id: &str, +/// Deploys the outcome of a relative-import relock as a new version of `head`, the path's live +/// version that the caller holds `FOR UPDATE`, and archives `head`. A `lock` of `None` records +/// a failed generation: the version carries `lock_error_logs` instead and runs keep resolving +/// to the last version that has a lock. A `modules` of `None` keeps the head's module locks. +/// +/// Writes whatever `head` names and checks nothing: callers are responsible for having +/// established access to its workspace and path, as a dependency job's push already has. +/// +/// `created_at` is stamped when the insert runs, not at transaction start. The row lock on +/// `head` is what orders one relock after another, and with `now()` a transaction that began +/// first but locked second commits a live child older than its archived parent, which every +/// "latest version" read then mis-orders. +pub async fn deploy_relocked_version( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + head: Script, deployment_message: Option, - db: &DB, -) -> crate::error::Result { - let mut tx = db.begin().await?; - let s = if let Some(s) = fetch_script_for_update(path, w_id, &mut *tx).await? { - s - } else { - return Err(crate::error::Error::NotFound(format!( - "Non-archived script with path '{}' not found", - path - ))); - }; + lock: Option<&str>, + modules: Option<&std::collections::HashMap>, + lock_error_logs: Option<&str>, +) -> crate::error::Result { + let s = head; + let w_id = s.workspace_id.as_str(); - let rs = runnable_settings::from_handle(s.runnable_settings.runnable_settings_handle, &mut *tx) - .await?; + let rs = + runnable_settings::from_handle(s.runnable_settings.runnable_settings_handle, &mut **tx) + .await?; let (debouncing_settings, concurrency_settings) = - runnable_settings::prefetch_cached_tx(&rs, &mut tx).await?; + runnable_settings::prefetch_cached_tx(&rs, &mut *tx).await?; + + // What the row stores is what the hash covers: the new module locks when there are any. + let modules = modules.cloned().or(s.modules); + let modules_json = modules.as_ref().map(serde_json::to_value).transpose()?; let ns = NewScript { path: s.path.clone(), @@ -392,7 +399,7 @@ pub async fn clone_script<'c>( content: s.content, schema: s.schema, is_template: s.is_template, - lock: None, + lock: lock.map(str::to_string), language: s.language, kind: Some(s.kind), tag: s.tag, @@ -424,7 +431,7 @@ pub async fn clone_script<'c>( on_behalf_of: s.on_behalf_of, preserve_on_behalf_of: None, assets: s.assets, - modules: s.modules, + modules, auto_parent: None, labels: s.labels, skip_draft_deletion: None, @@ -433,7 +440,7 @@ pub async fn clone_script<'c>( let new_hash = hash_script(&ns); tracing::debug!( - "cloning script at path {} from '{}' to '{}'", + "deploying relocked version of script at path {} from '{}' to '{}'", s.path, *s.hash, new_hash @@ -446,17 +453,19 @@ pub async fn clone_script<'c>( envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, \ dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, \ delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, \ - codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels) + codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels, \ + lock_error_logs, created_at) SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, \ - content, created_by, schema, is_template, extra_perms, NULL, language, kind, tag, \ + content, created_by, schema, is_template, extra_perms, $4::text, language, kind, tag, \ envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, \ dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, \ delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, \ - codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels + codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, COALESCE($5::jsonb, modules), labels, \ + $6::text, clock_timestamp() FROM script WHERE hash = $2 AND workspace_id = $3; - ", new_hash, s.hash.0, w_id).execute(&mut *tx).await?; + ", new_hash, s.hash.0, w_id, lock, modules_json, lock_error_logs).execute(&mut **tx).await?; // Archive base. sqlx::query!( @@ -464,9 +473,8 @@ pub async fn clone_script<'c>( *s.hash, w_id ) - .execute(&mut *tx) + .execute(&mut **tx) .await?; - tx.commit().await?; - Ok(ClonedScript { old_script: ns, new_hash }) + Ok(new_hash) } diff --git a/backend/windmill-dep-map/src/lib.rs b/backend/windmill-dep-map/src/lib.rs index 650e0e5f99..aab50795a9 100644 --- a/backend/windmill-dep-map/src/lib.rs +++ b/backend/windmill-dep-map/src/lib.rs @@ -128,6 +128,40 @@ pub fn extract_referenced_paths( } } +/// Re-records which paths `script_path` imports and what each one's lock hashes to right now. +/// That snapshot is what a later relock-skip check of this importer compares against, so it +/// has to move whenever the imports may have, whether or not the importer's own lock did. +/// +/// Writes for any path in `w_id` and checks nothing: callers are responsible for having +/// established access to that workspace and script, as a dependency job's push already has. +pub async fn refresh_dependency_map( + db: &sqlx::Pool, + w_id: &str, + script_path: &str, + parent_path: &Option, + code: &str, + script_lang: &Option, +) -> error::Result<()> { + use scoped_dependency_map::ScopedDependencyMap; + + let mut tx = db.begin().await?; + let mut dependency_map = + ScopedDependencyMap::fetch_maybe_rearranged(w_id, script_path, "script", parent_path, db) + .await?; + + tx = dependency_map + .patch( + extract_referenced_paths(code, script_path, *script_lang), + // Ideally should be None, but due to current implementation will use empty string to represent None. + "".into(), + tx, + ) + .await?; + + dependency_map.dissolve(tx).await.commit().await?; + Ok(()) +} + pub async fn process_relative_imports( db: &sqlx::Pool, _job_id: Option, @@ -145,29 +179,7 @@ pub async fn process_relative_imports( use scoped_dependency_map::ScopedDependencyMap; use trigger_dependents::trigger_dependents_to_recompute_dependencies; - // TODO: Should be moved into handle_dependency_job body to be more consistent with how flows and apps are handled - { - let mut tx = db.begin().await?; - let mut dependency_map = ScopedDependencyMap::fetch_maybe_rearranged( - &w_id, - script_path, - "script", - &parent_path, - db, - ) - .await?; - - tx = dependency_map - .patch( - extract_referenced_paths(&code, script_path, *script_lang), - // Ideally should be None, but due to current implementation will use empty string to represent None. - "".into(), - tx, - ) - .await?; - - dependency_map.dissolve(tx).await.commit().await?; - } + refresh_dependency_map(db, w_id, script_path, &parent_path, code, script_lang).await?; { let mut already_visited = args diff --git a/backend/windmill-queue/src/jobs.rs b/backend/windmill-queue/src/jobs.rs index c7c119a5a7..8ac4fd390a 100644 --- a/backend/windmill-queue/src/jobs.rs +++ b/backend/windmill-queue/src/jobs.rs @@ -4007,33 +4007,10 @@ async fn clone_runnable(j: &mut PulledJob, db: &DB) -> error::Result<()> { { let maybe_new_id = match j.kind { - JobKind::Dependencies => { - let deployment_message = j - .args - .clone() - .map(|hashmap| { - hashmap - .get("deployment_message") - .map(|map_value| serde_json::from_str::(map_value.get()).ok()) - .flatten() - }) - .flatten(); - - // This way we tell downstream which script we should archive when the resolution is finished. - // (not used at the moment) - j.args - .as_mut() - .map(|args| args.insert("base_hash".to_owned(), to_raw_value(&*base_hash))); - - windmill_common::scripts::clone_script( - j.runnable_path(), - &j.workspace_id, - deployment_message, - db, - ) - .await? - .new_hash - } + // A script gets its new version from the worker, once the generated lock is known + // to differ from the live version's: minting one here would deploy, and walk the + // importers of, a version whose lock turns out byte-identical to its parent's. + JobKind::Dependencies => *base_hash, JobKind::FlowDependencies => { sqlx::query_scalar!( "INSERT INTO flow_version diff --git a/backend/windmill-types/src/scripts.rs b/backend/windmill-types/src/scripts.rs index 414650e846..19f6a06021 100644 --- a/backend/windmill-types/src/scripts.rs +++ b/backend/windmill-types/src/scripts.rs @@ -16,7 +16,7 @@ use crate::{ runnable_settings::{ConcurrencySettings, DebouncingSettings}, }; -#[derive(Serialize, Deserialize, Debug, Clone, Hash)] +#[derive(Serialize, Deserialize, Debug, Clone, Hash, PartialEq, Eq)] pub struct ScriptModule { pub content: String, pub language: ScriptLang, diff --git a/backend/windmill-worker/src/worker_lockfiles.rs b/backend/windmill-worker/src/worker_lockfiles.rs index 57b859075a..9bba9a99e6 100644 --- a/backend/windmill-worker/src/worker_lockfiles.rs +++ b/backend/windmill-worker/src/worker_lockfiles.rs @@ -19,14 +19,17 @@ use windmill_common::error::Result; use windmill_common::flows::{FlowModule, FlowModuleValue, FlowNodeId}; use windmill_common::jobs::JobKind; use windmill_common::min_version::MIN_VERSION_SUPPORTS_DEBOUNCING_V2; -use windmill_common::scripts::ScriptHash; +use windmill_common::scripts::{ + deploy_relocked_version, fetch_script_for_update, hash_script, ScriptHash, ScriptModule, +}; #[cfg(feature = "python")] use windmill_common::worker::PythonAnnotations; use windmill_common::worker::{to_raw_value, to_raw_value_owned, write_file, Connection}; use windmill_common::workspace_dependencies::{ RawWorkspaceDependencies, WorkspaceDependenciesPrefetched, }; -use windmill_dep_map::scoped_dependency_map::ScopedDependencyMap; +use windmill_dep_map::scoped_dependency_map::{DependencyDependent, ScopedDependencyMap}; +use windmill_dep_map::trigger_dependents::trigger_dependents_to_recompute_dependencies; #[cfg(feature = "python")] use windmill_parser_yaml::AnsibleRequirements; @@ -38,8 +41,10 @@ use windmill_common::{ scripts::ScriptLang, DB, }; +use windmill_dep_map::lock_hash::record_lock_hashes; pub use windmill_dep_map::{ extract_referenced_paths, extract_relative_imports, process_relative_imports, + refresh_dependency_map, }; use windmill_git_sync::{ handle_deployment_metadata, tally_deployed_object_changes, DeployedObject, @@ -86,10 +91,13 @@ use crate::{ /// has the toolchain and, since the cache key is per OS/arch, the platform the runtime /// workers use. Deploys that supply their own lock never reach a dependency job at all and /// queue theirs from `create_script_internal` instead. -async fn maybe_queue_binary_prebuild(db: &DB, job: &MiniPulledJob, lock: &str) -> Result<()> { - let (Some(hash), Some(path), Some(lang)) = - (job.runnable_id, job.runnable_path.clone(), job.script_lang) - else { +async fn maybe_queue_binary_prebuild( + db: &DB, + job: &MiniPulledJob, + hash: ScriptHash, + lock: &str, +) -> Result<()> { + let (Some(path), Some(lang)) = (job.runnable_path.clone(), job.script_lang) else { return Ok(()); }; let Some(prebuild) = @@ -283,6 +291,13 @@ pub async fn handle_dependency_job( job.runnable_path() ); let script_path = job.runnable_path(); + let w_id = &job.workspace_id; + + let triggered_by_relative_import = job + .args + .as_ref() + .map(|x| x.get("triggered_by_relative_import").is_some()) + .unwrap_or_default(); // A build pass reads the same script data but writes none of the deploy state below, // including the `lock_error_logs` stamp on a fetch failure: the version it builds is @@ -296,14 +311,48 @@ pub async fn handle_dependency_job( *deployment_tallied = true; } + // A relative-import relock deploys nothing until `commit_relock` says so, while the + // caller's fallback tally assumes a failed dependency job left a deployed version behind. + // Claim the tally here; the failure path hands it back once it has minted the version + // that carries the error. + if triggered_by_relative_import { + *deployment_tallied = true; + } + + // A relative-import relock locks the path's live version as of now, not the hash captured + // when the job was pushed: a deploy can land during the debounce delay, after which that + // hash names an archived version. What it generates is committed against the live version + // re-read under a row lock, so a deploy landing mid-generation is caught there too. + let target_hash = if triggered_by_relative_import { + Some(ScriptHash(live_head_hash(db, w_id, script_path).await?)) + } else { + job.runnable_id + }; + // `JobKind::Dependencies` job store either: // - A saved script `hash` in the `script_hash` column. // - Preview raw lock and code in the `queue` or `job` table. - let script_data = &match job.runnable_id { + let script_data = &match target_hash { + // Read straight from the database: the cache pins a version's data under its hash for + // as long as this worker lives, and the live version may still be waiting on its own + // dependency job's lock, which lands in place. A run resolving to it on this worker + // would then get no lock from the cache at all. + Some(hash) if triggered_by_relative_import => { + let raw = cache::script::fetch_script_from_db(db, hash, std::panic::Location::caller()) + .await?; + Cow::Owned(std::sync::Arc::new(cache::ScriptData { + lock: raw.lock, + code: raw.content, + modules: raw.modules, + })) + } Some(hash) => match cache::script::fetch(&Connection::from(db.clone()), hash).await { Ok(d) => Cow::Owned(d.0), Err(e) => { - if !is_build_job { + // The live version of a relative-import relock is what runs resolve to, and + // `lock_error_logs` on it takes it out of resolution; the job carries the + // error instead, since it deployed nothing. + if !is_build_job && !triggered_by_relative_import { let logs2 = sqlx::query_scalar!( "SELECT logs FROM job_logs WHERE job_id = $1 AND workspace_id = $2", &job.id, @@ -348,12 +397,6 @@ pub async fn handle_dependency_job( .await; } - let triggered_by_relative_import = job - .args - .as_ref() - .map(|x| x.get("triggered_by_relative_import").is_some()) - .unwrap_or_default(); - // Extract temp_script_refs from job args (path -> hash mapping for temp storage) let temp_script_refs: Option> = job .args @@ -391,20 +434,17 @@ pub async fn handle_dependency_job( ) .await; + let (deployment_message, parent_path) = + get_deployment_msg_and_parent_path_from_args(job.args.clone()); + match content { Ok(content) => { - if job.runnable_id.is_none() { + let Some(current_hash) = target_hash else { // it a one-off raw script dependency job, no need to update the db return Ok(to_raw_value_owned( json!({ "status": "Successful lock file generation", "lock": content }), )); - } - - let current_hash = job.runnable_id.unwrap_or(ScriptHash(0)); - let w_id = &job.workspace_id; - - let (deployment_message, parent_path) = - get_deployment_msg_and_parent_path_from_args(job.args.clone()); + }; // Generate lockfiles for module files (if any). // @@ -441,7 +481,9 @@ pub async fn handle_dependency_job( occupancy_metrics, &raw_workspace_dependencies_o, module.lock.as_deref(), - triggered_by_relative_import, + // A module that was never locked has nothing a skip could hand + // back; the path's lock is the parent script's, not its own. + triggered_by_relative_import && module.lock.is_some(), script_path, None, "script", @@ -464,34 +506,83 @@ pub async fn handle_dependency_job( None }; - // We do not create new row for this update - // That means we can keep current hash and just update lock - // Also store lockfile hash for dependency change detection - let lockfile_hash = windmill_common::scripts::hash_script(&content); - let updated_modules_json = updated_modules - .as_ref() - .and_then(|m| serde_json::to_value(m).ok()); - sqlx::query!( - "WITH update_lock AS ( - UPDATE script SET lock = $1, modules = COALESCE($6, modules) WHERE hash = $2 AND workspace_id = $3 + let deployed_hash = if triggered_by_relative_import { + match commit_relock( + db, + w_id, + script_path, + current_hash, + Some(&content), + updated_modules.as_ref(), + None, + deployment_message.clone(), ) - INSERT INTO lock_hash (workspace_id, path, lockfile_hash) - VALUES ($3, $4, $5) - ON CONFLICT (workspace_id, path) DO UPDATE SET lockfile_hash = $5", - &content, - ¤t_hash.0, - w_id, - script_path, - &lockfile_hash, - updated_modules_json - ) - .execute(db) - .await?; + .await? + { + RelockOutcome::Deployed(hash) => hash, + RelockOutcome::Unchanged => { + let log_msg = "\nLock unchanged: no new version deployed"; + tracing::info!(workspace_id = %w_id, job_id = %job.id, "{log_msg}"); + append_logs(&job.id, w_id, log_msg, &db.into()).await; + // The imports may have moved even though the result did not, and the + // map is what this importer's next skip check reads. + refresh_dependency_map( + db, + w_id, + script_path, + &parent_path, + &script_data.code, + &job.script_lang, + ) + .await?; + return Ok(to_raw_value_owned( + json!({ "status": "Lock unchanged, no new version deployed", "lock": content }), + )); + } + RelockOutcome::Superseded(head) => { + let log_msg = format!( + "\nVersion {head} was deployed while this lock was generated; discarding it and queueing a relock of that version" + ); + tracing::info!(workspace_id = %w_id, job_id = %job.id, "{log_msg}"); + append_logs(&job.id, w_id, log_msg, &db.into()).await; + requeue_relock(db, job, script_path, deployment_message, parent_path) + .await?; + return Ok(to_raw_value_owned( + json!({ "status": "Lock generation superseded by a newer version", "lock": content }), + )); + } + } + } else { + // We do not create new row for this update + // That means we can keep current hash and just update lock + // Also store lockfile hash for dependency change detection + let lockfile_hash = windmill_common::scripts::hash_script(&content); + let updated_modules_json = updated_modules + .as_ref() + .and_then(|m| serde_json::to_value(m).ok()); + sqlx::query!( + "WITH update_lock AS ( + UPDATE script SET lock = $1, modules = COALESCE($6, modules) WHERE hash = $2 AND workspace_id = $3 + ) + INSERT INTO lock_hash (workspace_id, path, lockfile_hash) + VALUES ($3, $4, $5) + ON CONFLICT (workspace_id, path) DO UPDATE SET lockfile_hash = $5", + &content, + ¤t_hash.0, + w_id, + script_path, + &lockfile_hash, + updated_modules_json + ) + .execute(db) + .await?; - // `lock` has been updated; invalidate the cache. - // Since only worker that ran this Dependency Job has the cache - // we do not need to think about invalidating cache for other workers. - cache::script::invalidate(current_hash); + // `lock` has been updated; invalidate the cache. + // Since only worker that ran this Dependency Job has the cache + // we do not need to think about invalidating cache for other workers. + cache::script::invalidate(current_hash); + current_hash + }; // The version only became runnable now, so this process still resolves the path to // the one before it. Only the runnable-hash cache: the import-side caches ignore the // lock, so evicting this process' half of that pair here would key a bundle by a @@ -504,7 +595,7 @@ pub async fn handle_dependency_job( &db, &w_id, DeployedObject::Script { - hash: current_hash, + hash: deployed_hash, path: script_path.to_string(), parent_path: parent_path.clone(), }, @@ -565,7 +656,7 @@ pub async fn handle_dependency_job( }); } - if let Err(e) = maybe_queue_binary_prebuild(db, job, &content).await { + if let Err(e) = maybe_queue_binary_prebuild(db, job, deployed_hash, &content).await { tracing::error!(%e, "error queueing the auto-build binary job for {script_path}"); } @@ -583,14 +674,49 @@ pub async fn handle_dependency_job( .await? .flatten() .unwrap_or_else(|| "no logs".to_string()); - sqlx::query!( - "UPDATE script SET lock_error_logs = $1 WHERE hash = $2 AND workspace_id = $3", - &format!("{logs2}\n{error}"), - &job.runnable_id.unwrap_or(ScriptHash(0)).0, - &job.workspace_id - ) - .execute(db) - .await?; + let error_logs = format!("{logs2}\n{error}"); + if let (true, Some(hash)) = (triggered_by_relative_import, target_hash) { + // The same shape a failed deploy leaves: a version without a lock that carries + // the error, so it shows on the script while runs keep resolving to the last + // version that has one. Only that version is the caller's fallback to tally; + // one that landed meanwhile owns its own lock, and a commit that failed left + // nothing. + match commit_relock( + db, + w_id, + script_path, + hash, + None, + None, + Some(&error_logs), + deployment_message.clone(), + ) + .await + { + Ok(RelockOutcome::Deployed(_)) => *deployment_tallied = false, + Ok(RelockOutcome::Superseded(_)) => { + if let Err(e) = + requeue_relock(db, job, script_path, deployment_message, parent_path) + .await + { + tracing::error!(%e, "error queueing a relock of {script_path}") + } + } + Ok(RelockOutcome::Unchanged) => {} + Err(e) => { + tracing::error!(%e, "error recording the failed relock of {script_path}") + } + } + } else { + sqlx::query!( + "UPDATE script SET lock_error_logs = $1 WHERE hash = $2 AND workspace_id = $3", + &error_logs, + &job.runnable_id.unwrap_or(ScriptHash(0)).0, + &job.workspace_id + ) + .execute(db) + .await?; + } Err(Error::ExecutionErr(format!( "Error locking file: {error}\n\nlogs:\n{}", remove_ansi_codes(&logs2) @@ -598,6 +724,136 @@ pub async fn handle_dependency_job( } } } + +/// The version of `script_path` that runs resolve to, which is what a relative-import relock +/// locks. `NotFound` when the path holds none, which a job pushed for a path since archived or +/// deleted reports as its own failure. +async fn live_head_hash(db: &DB, w_id: &str, script_path: &str) -> error::Result { + sqlx::query_scalar!( + "SELECT hash FROM script WHERE path = $1 AND workspace_id = $2 AND deleted = false AND archived = false ORDER BY created_at DESC LIMIT 1", + script_path, + w_id + ) + .fetch_optional(db) + .await? + .ok_or_else(|| { + Error::NotFound(format!( + "Non-archived script with path '{script_path}' not found" + )) + }) +} + +enum RelockOutcome { + /// A new version carrying the result is the live one. + Deployed(ScriptHash), + /// The live version already holds this lock and these module locks; nothing was written. + Unchanged, + /// The live version is no longer the one the lock was generated for; nothing was written. + Superseded(ScriptHash), +} + +/// Commits what a relative-import relock produced against the path's live version, read under +/// a row lock so relocks of one path serialize on it. +/// +/// A result equal to the live version's lock and module locks writes nothing: the importer's +/// dependencies did not move, and a new version would deploy byte-identical content and then +/// walk its own importers for nothing. A `lock` of `None` is a failed generation and always +/// deploys, as the version that carries the error. +async fn commit_relock( + db: &DB, + w_id: &str, + script_path: &str, + generated_for: ScriptHash, + lock: Option<&str>, + modules: Option<&HashMap>, + lock_error_logs: Option<&str>, + deployment_message: Option, +) -> error::Result { + let mut tx = db.begin().await?; + let mut head = None; + for _ in 0..4 { + head = fetch_script_for_update(script_path, w_id, &mut *tx).await?; + if head.is_some() { + break; + } + // Having waited on the live version's row lock, the statement re-checked that row + // once the holder committed, found it archived, and returned nothing: the successor + // the holder inserted is not in the statement's snapshot. A fresh statement sees it, + // unless yet another writer got there first, so this goes around a few times before + // concluding the path holds no live version. + } + let Some(head) = head else { + return Err(Error::NotFound(format!( + "Non-archived script with path '{script_path}' not found" + ))); + }; + if head.hash != generated_for { + // A deploy landed while the lock was generated. It carried its own lock or queued its + // own dependency job, and this lock describes content that is no longer live. + return Ok(RelockOutcome::Superseded(head.hash)); + } + let lock_hash_entry = lock.map(|lock| (script_path.to_string(), hash_script(lock))); + if let Some(lock) = lock { + let modules_unchanged = modules.map_or(true, |m| head.modules.as_ref() == Some(m)); + if head.lock.as_deref() == Some(lock) && modules_unchanged { + // The hash row is still written, and under the same row lock: a version deployed + // before lock hashes were recorded has none, so its importers cannot skip until + // it does, and a deploy that takes the lock next must not have the hash it records + // overwritten by this one. + record_lock_hashes(&mut tx, w_id, lock_hash_entry.as_slice()).await?; + tx.commit().await?; + return Ok(RelockOutcome::Unchanged); + } + } + let new_hash = deploy_relocked_version( + &mut tx, + head, + deployment_message, + lock, + modules, + lock_error_logs, + ) + .await?; + record_lock_hashes(&mut tx, w_id, lock_hash_entry.as_slice()).await?; + tx.commit().await?; + Ok(RelockOutcome::Deployed(ScriptHash(new_hash))) +} + +/// Queues another relative-import relock of `script_path`, through the same push the fan-out +/// uses. The version live now was deployed while a lock was generated for its predecessor; +/// when that deploy was a sibling relock it queued nothing for this path, and the result just +/// discarded may have been the one generated against the current imports. +async fn requeue_relock( + db: &DB, + job: &MiniPulledJob, + script_path: &str, + deployment_message: Option, + parent_path: Option, +) -> error::Result<()> { + let already_visited = job + .args + .as_ref() + .and_then(|x| x.get("already_visited")) + .and_then(|v| serde_json::from_str::>(v.get()).ok()) + .unwrap_or_default(); + trigger_dependents_to_recompute_dependencies( + &job.workspace_id, + vec![DependencyDependent { + importer_path: script_path.to_string(), + importer_kind: "script".to_string(), + importer_node_ids: None, + }], + deployment_message, + parent_path, + &job.permissioned_as_email, + &job.created_by, + &job.permissioned_as, + db, + already_visited, + ) + .await +} + fn remove_ansi_codes(s: &str) -> String { lazy_static::lazy_static! { static ref ANSI_REGEX: regex::Regex = regex::Regex::new(r"\x1b\[[0-9;]*[a-zA-Z]").unwrap(); @@ -2892,8 +3148,12 @@ async fn try_skip_relock( } // Fetch existing lock based on runnable type - let lock = match runnable_type { - "script" => sqlx::query_scalar!( + let lock = match (runnable_type, existing_lock) { + // A script's module asks with the script's own type and hands over the lock it last + // deployed with. The path's lock below is the parent script's, and a module given + // that loses whatever it resolves on its own. + ("script", Some(module_lock)) => Some(module_lock.to_string()), + ("script", None) => sqlx::query_scalar!( "SELECT lock FROM script WHERE path = $1 AND workspace_id = $2 AND lock IS NOT NULL AND deleted = false ORDER BY created_at DESC LIMIT 1", base_path, @@ -2903,7 +3163,7 @@ async fn try_skip_relock( .await? .flatten(), - "flow" | "app" => existing_lock.map(|s| s.to_string()), + ("flow" | "app", existing_lock) => existing_lock.map(|s| s.to_string()), _ => None, }; From 0d6bce4a12226adb83fc6050064eb2e80372e36e Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 3 Sep 2026 23:23:59 +0000 Subject: [PATCH 006/130] keep the SSO group reconciler alive in oauth2-less builds (#10969) * chore: stop denying reads of secret files in claude settings Any Read() deny rule makes Claude Code resolve the file operands of every Bash command that reads files. A path it cannot resolve, such as one that follows a cd into a directory the analyzer does not track, escalates to a permission prompt even under bypassPermissions. A plain recursive grep in the repo root escalates too, because it could reach .env. Drop the read rules and widen the write rules to cover the same files, so secrets still cannot be written through Edit, Write, or a shell redirect. Reads of those files are no longer blocked. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RNCupPk2yewQT1JMNjkV8M * fix: keep the sso group reconciler alive in oauth2-less builds Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01W24T1FjQXQ87AoeC3UxWWC * chore: update ee-repo-ref to d6297e6844dc2aab4745fce328e32ccab508969f This commit updates the EE repository reference after PR #777 was merged in windmill-ee-private. Previous ee-repo-ref: eec88486fb2df0ba15998ef285f52fc67af90b1e New ee-repo-ref: d6297e6844dc2aab4745fce328e32ccab508969f Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: windmill-internal-app[bot] --- backend/ee-repo-ref.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 354e88bef1..9e7a10edf0 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -3b89bfc11314a326a191101cfe3ef65f6f7f82a8 +d6297e6844dc2aab4745fce328e32ccab508969f From fda7b3f086619e3716e5894c07be127104174f1d Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 4 Sep 2026 08:53:02 +0000 Subject: [PATCH 007/130] feat(ai-sessions): replace the context panel with an assistant settings modal (#10919) * feat(ai-chat): make reusable skills ai_skill resources you select per workspace * chore: pin the ee ref to the skill telemetry counters * fix: address review findings on skill authoring, import and migration * fix: enforce skill selection in read_skill and stop imports clobbering resources * feat: carry format_extension from the hub into synced resource types * fix: let an edit set or clear a resource type's format_extension * fix: regenerate the sqlx cache and close the review round findings * fix: close the round-2 findings on folder ACLs, cached sync and truncation * refactor: make the skills migration non-destructive and use design-system inputs * feat(ai-sessions): add a context panel listing what the chat can use Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NmdCVM1ZvTcpatv78jN8Ed * fix: track the prompt rebuild signal and trim the review round's nits Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NmdCVM1ZvTcpatv78jN8Ed * fix: keep the panel from perturbing an in-flight turn Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NmdCVM1ZvTcpatv78jN8Ed * fix: count a folder by its readable children Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NmdCVM1ZvTcpatv78jN8Ed * feat(ai-sessions): replace the context panel with an assistant settings modal Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * feat(ai-sessions): page-based MCP editing and fuzzy tool search Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * feat(ai-sessions): tool detail page and a shared list row Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * feat(ai-sessions): add a files & folders section to the assistant settings Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * chore: point the ee ref at the merged ee branch Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * fix: keep hidden sections from answering keys and swallowing a failed save Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * fix: restore the staged-fork write guard and narrow the round-2 findings Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * fix: restore the workspace-race guards and extend them to MCP Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * fix: keep an in-flight settings read from overwriting typed instructions Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * docs: describe the tool row as the one line it renders Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * fix: keep the prompt entries on the home composer, which has no settings modal Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * fix: create the editor with the gutter its caller asked for Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * test: restore the attachment status label guard dropped in the merge Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * fix: surface a refused mcp selection write instead of painting the switch Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * fix: refuse instruction writes to a staged fork's parent, and read the target's role Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * fix: pin the instructions role and field to the target workspace Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Ld1m9NAGLdPPrNBiu5PSQK * fix: retry a deferred instructions reload, and use Button for the row label Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WGnKFvaX61wiMU1CzQp6XG * fix: leave the arrows to a control that answered them, and say when a role read failed Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WGnKFvaX61wiMU1CzQp6XG * chore: update ee-repo-ref to a2776856c50e80c9dbcf6e689a66ce86567c03fa This commit updates the EE repository reference after PR #765 was merged in windmill-ee-private. Previous ee-repo-ref: dd7466e749753568a23c91ba5e165020769206b8 New ee-repo-ref: a2776856c50e80c9dbcf6e689a66ce86567c03fa Automated by sync-ee-ref workflow. * fix: withhold the page navigator from a parked section Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WGnKFvaX61wiMU1CzQp6XG --------- Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Guilhem Lemouel Co-authored-by: windmill-internal-app[bot] --- .../src/lib/components/AppConnectInner.svelte | 136 +-- .../src/lib/components/ResourceEditor.svelte | 6 +- frontend/src/lib/components/Section.svelte | 2 +- .../src/lib/components/SimpleEditor.svelte | 31 +- .../common/fileInput/FileInput.svelte | 7 + frontend/src/lib/components/common/index.ts | 1 + .../components/common/listRow/ListRow.svelte | 142 +++ .../common/listRow/listHighlight.svelte.ts | 84 ++ .../lib/components/common/modal/Modal2.svelte | 7 +- .../common/modal/PagedContent.svelte | 6 + .../common/sidebar/SidebarNavigation.svelte | 6 +- .../copilot/chat/AIChatDisplay.svelte | 24 +- .../copilot/chat/AIChatManager.svelte.ts | 5 +- .../copilot/chat/AIChatModelSettings.svelte | 44 +- .../copilot/chat/AssistantFilesSection.svelte | 158 ++++ .../chat/AssistantInstructionsSection.svelte | 354 ++++++++ .../copilot/chat/AssistantMcpSection.svelte | 632 +++++++++++++ .../chat/AssistantSettingsModal.svelte | 238 +++++ ...r.svelte => AssistantSkillsSection.svelte} | 859 ++++++++++-------- .../copilot/chat/AssistantToolsSection.svelte | 212 +++++ .../copilot/chat/ChatCollapsibleCard.svelte | 1 + .../copilot/chat/McpConnections.svelte | 347 ------- .../copilot/chat/agentContext.test.ts | 65 ++ .../components/copilot/chat/agentContext.ts | 58 ++ .../copilot/chat/skills/skillsMenu.svelte.ts | 144 +++ .../src/lib/components/mcp/McpConnect.svelte | 24 +- .../src/lib/components/mcp/mcpMenu.svelte.ts | 197 ++++ .../src/lib/components/mcp/secretVariable.ts | 2 +- .../sessions/sessionRuntime.svelte.ts | 6 + 29 files changed, 2923 insertions(+), 875 deletions(-) create mode 100644 frontend/src/lib/components/common/listRow/ListRow.svelte create mode 100644 frontend/src/lib/components/common/listRow/listHighlight.svelte.ts create mode 100644 frontend/src/lib/components/copilot/chat/AssistantFilesSection.svelte create mode 100644 frontend/src/lib/components/copilot/chat/AssistantInstructionsSection.svelte create mode 100644 frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte create mode 100644 frontend/src/lib/components/copilot/chat/AssistantSettingsModal.svelte rename frontend/src/lib/components/copilot/chat/{SkillsPicker.svelte => AssistantSkillsSection.svelte} (50%) create mode 100644 frontend/src/lib/components/copilot/chat/AssistantToolsSection.svelte delete mode 100644 frontend/src/lib/components/copilot/chat/McpConnections.svelte create mode 100644 frontend/src/lib/components/copilot/chat/agentContext.test.ts create mode 100644 frontend/src/lib/components/copilot/chat/agentContext.ts create mode 100644 frontend/src/lib/components/copilot/chat/skills/skillsMenu.svelte.ts create mode 100644 frontend/src/lib/components/mcp/mcpMenu.svelte.ts diff --git a/frontend/src/lib/components/AppConnectInner.svelte b/frontend/src/lib/components/AppConnectInner.svelte index a9a0837d1f..3d73cea72f 100644 --- a/frontend/src/lib/components/AppConnectInner.svelte +++ b/frontend/src/lib/components/AppConnectInner.svelte @@ -21,9 +21,10 @@ } from '$lib/gen' import { emptyString, truncateRev, urlize } from '$lib/utils' import { registryEntryFor, registryCcCapableFor, stripSandboxSuffix } from './oauthRegistry' - import { createEventDispatcher, onDestroy, tick, untrack } from 'svelte' + import { createEventDispatcher, onDestroy, tick } from 'svelte' import Path from './Path.svelte' - import { Button, RadioCard, Skeleton } from './common' + import { ListRow, RadioCard, Skeleton } from './common' + import { useListHighlight } from './common/listRow/listHighlight.svelte' import ApiConnectForm from './ApiConnectForm.svelte' import SearchItems from './SearchItems.svelte' import WhitelistIp from './WhitelistIp.svelte' @@ -42,7 +43,6 @@ import SyncResourceTypes from './SyncResourceTypes.svelte' import Label from './Label.svelte' import ResourcePathHint from './ResourcePathHint.svelte' - import { twMerge } from 'tailwind-merge' interface Props { step?: number @@ -1027,15 +1027,8 @@ // Both lists start undefined and render skeletons; "nothing found" only means something // once they have landed. let listsLoaded = $derived(rankedConnectsManual !== undefined && rankedConnects !== undefined) - let highlightedIndex = $state(-1) const rowDomId = (index: number) => `resource-type-row-${index}` - // Set at hover time rather than up front, so only the descriptions the row actually cut - // off carry a tooltip. - function titleIfTruncated(e: MouseEvent & { currentTarget: HTMLElement }) { - const el = e.currentTarget - el.title = el.scrollWidth > el.clientWidth ? (el.textContent?.trim() ?? '') : '' - } const oauthRowOffset = $derived(customKeys.length) const otherRowOffset = $derived(customKeys.length + (rankedConnects?.length ?? 0)) @@ -1054,53 +1047,23 @@ return best } - // Filtering reshuffles the rows under the highlight: point it at the best match so Enter - // takes the top hit, and drop it entirely once the filter is cleared. - $effect(() => { - navItems - filter - untrack(() => (highlightedIndex = searching ? bestMatchIndex() : -1)) + const highlight = useListHighlight({ + count: () => navItems.length, + rowId: rowDomId, + // Sections are rendered in a fixed order, so the best match is not necessarily the + // first row; Enter should still take the top hit. + restingIndex: () => (searching ? bestMatchIndex() : -1), + onActivate: (index) => { + const item = navItems[index] + if (!item) return + item.oauth ? connectOauth(item.key) : selectFromOthers(item.key) + }, + activateEnterFrom: [SEARCH_INPUT_ID] }) - // Scrolling rows under a resting pointer makes the browser fire `mouseenter` on each one, - // which would drag the highlight back under the cursor as the arrow keys move it. Only a - // real pointer move hands the highlight back to the mouse. - let pointerOwnsHighlight = $state(true) - - function highlightHovered(index: number) { - if (pointerOwnsHighlight) highlightedIndex = index - } - - function moveHighlight(delta: number) { - const count = navItems.length - if (count === 0) return - pointerOwnsHighlight = false - // Rows are tabbable buttons, so focus can sit on one. Enter then activates whatever is - // focused, which has to stay the highlighted row. - const rowWasFocused = document.activeElement?.id?.startsWith('resource-type-row-') ?? false - highlightedIndex = - highlightedIndex < 0 - ? delta > 0 - ? 0 - : count - 1 - : (highlightedIndex + delta + count) % count - const row = document.getElementById(rowDomId(highlightedIndex)) - row?.scrollIntoView({ block: 'nearest' }) - if (rowWasFocused) row?.focus() - } - function onListKeydown(e: KeyboardEvent) { if (step !== 1) return - if (e.key === 'ArrowDown' || e.key === 'ArrowUp') { - e.preventDefault() - moveHighlight(e.key === 'ArrowDown' ? 1 : -1) - } else if (e.key === 'Enter' && (e.target as HTMLElement)?.id === SEARCH_INPUT_ID) { - // A focused row activates itself on Enter; this covers Enter typed in the search field. - const item = navItems[highlightedIndex] - if (!item) return - e.preventDefault() - item.oauth ? connectOauth(item.key) : selectFromOthers(item.key) - } + highlight.onKeydown(e) } let editScopes = $state(false) @@ -1132,7 +1095,7 @@
    (pointerOwnsHighlight = true)} + onpointermove={highlight.pointerMoved} >
    @@ -1146,28 +1109,6 @@
    - {#snippet resourceRow(key: string)} -
    -
    - -
    -
    -
    - {resourceTypeDisplayName(key)} - {key} -
    - {#if resourceTypeDescriptions[key]} - - {plainDescription(resourceTypeDescriptions[key])} - - {/if} -
    -
    - {/snippet} - {#snippet sectionHeading(title: string, count: number)}

    {title}{#if searching}{count}{/if} @@ -1175,26 +1116,29 @@ {/snippet} {#snippet resourceButton(key: string, index: number, oauth: boolean)} - + {icon} + {title} + subtitle={resourceTypeDescriptions[key] ? subtitle : undefined} + highlighted={index === highlight.index} + onMouseEnter={() => highlight.hovered(index)} + onClick={() => (oauth ? connectOauth(key) : selectFromOthers(key))} + /> {/snippet}
    @@ -1213,7 +1157,7 @@ {#if customKeys.length > 0}
    {@render sectionHeading('Custom resource types', customKeys.length)} -
    +
    {#each customKeys as key, i} {@render resourceButton(key, i, false)} {/each} @@ -1227,7 +1171,7 @@ 'Instance-configured OAuth APIs', rankedConnects?.length ?? 0 )} -
    +
    {#if rankedConnects} {#each rankedConnects as { key }, i} {@render resourceButton(key, oauthRowOffset + i, true)} @@ -1259,7 +1203,7 @@
    {/if} -
    +
    {#if rankedConnectsManual} {#each otherKeys as key, i} {@render resourceButton(key, otherRowOffset + i, false)} diff --git a/frontend/src/lib/components/ResourceEditor.svelte b/frontend/src/lib/components/ResourceEditor.svelte index 367888655f..7b5cbacb5d 100644 --- a/frontend/src/lib/components/ResourceEditor.svelte +++ b/frontend/src/lib/components/ResourceEditor.svelte @@ -320,7 +320,9 @@ current.path = npath } - export async function save(): Promise { + /** Whether the write landed. It toasts its own failure, so most callers ignore this; + * one that follows the save with bookkeeping of its own has to know not to. */ + export async function save(): Promise { const dirty = dirtyWorkspaces try { for (const ws of dirty) { @@ -368,8 +370,10 @@ dirty.length > 1 ? `Saved resource in ${dirty.length} workspaces` : `Saved resource` ) dispatch('refresh', current?.path ?? path) + return true } catch (err) { sendUserToast(`Could not save resource: ${err.body ?? err.message}`, true) + return false } } diff --git a/frontend/src/lib/components/Section.svelte b/frontend/src/lib/components/Section.svelte index f1425c51b7..badc7c9531 100644 --- a/frontend/src/lib/components/Section.svelte +++ b/frontend/src/lib/components/Section.svelte @@ -102,7 +102,7 @@ transition:slide={animate || collapsable ? { duration: 200 } : { duration: 0 }} > {#if description} -
    {@html description}
    +
    {@html description}
    {/if}
    diff --git a/frontend/src/lib/components/SimpleEditor.svelte b/frontend/src/lib/components/SimpleEditor.svelte index 65713e19f7..8133494f18 100644 --- a/frontend/src/lib/components/SimpleEditor.svelte +++ b/frontend/src/lib/components/SimpleEditor.svelte @@ -63,6 +63,10 @@ /** Trailing debounce window (ms) on Monaco's onDidChangeModelContent. */ const CHANGE_TIMEOUT = 200 + /** Gap between the line numbers and the first character. Zero puts them flush, + * so a two-digit line reads as one token with the code. */ + const LINE_DECORATIONS_WIDTH = 6 + let changeTimeoutId: number | undefined = undefined // Monaco fires onDidChangeModelContent synchronously from within `setValue`, so without // this an authoritative overwrite reads as a user edit on the `input` event. @@ -112,7 +116,8 @@ minHeight = 1000, renderLineHighlight = 'none', suggestion, - leadingChangeSync = false + leadingChangeSync = false, + lineNumbersMinChars = 3 }: { lang: string code?: string @@ -149,6 +154,9 @@ * `code`; leave it off where each extra sync costs work downstream (an app * code input feeding an autoRefresh runnable re-runs a job per sync). */ leadingChangeSync?: boolean + /** Width of the line-number gutter, in characters. Same name, and same + * default, as `Editor`, so the two render line numbers alike. */ + lineNumbersMinChars?: number } = $props() let yPadding = MONACO_Y_PADDING @@ -312,10 +320,12 @@ if (model.getLanguageId() !== lang) { const currentCode = model.getValue() const uri = `file:///${hash}.${langToExt(lang)}` - const oldModel = model - const newModel = meditor.createModel(currentCode, lang, mUri.parse(uri)) - editor?.setModel(newModel) - oldModel.dispose() + // The old model goes first: `langToExt` maps anything it does not know to + // `unknown`, so the new uri is usually the one this model already holds, + // and creating over an occupied uri throws ("model already exists"). + editor?.setModel(null) + model.dispose() + editor?.setModel(meditor.createModel(currentCode, lang, mUri.parse(uri))) } // Update editor options for suggestions, validation decorations, and line numbers @@ -334,8 +344,8 @@ snippetsPreventQuickSuggestions: disableSuggestions }, lineNumbers: hideLineNumbers ? 'off' : 'on', - lineDecorationsWidth: hideLineNumbers ? 0 : 6, - lineNumbersMinChars: hideLineNumbers ? 0 : 2, + lineDecorationsWidth: hideLineNumbers ? 0 : LINE_DECORATIONS_WIDTH, + lineNumbersMinChars: hideLineNumbers ? 0 : lineNumbersMinChars, // Hide validation squiggles and decorations renderValidationDecorations: disableLinting ? 'off' : 'on', // Hide the validation margin indicators @@ -397,8 +407,11 @@ ...(yPadding !== undefined ? { padding: { bottom: yPadding, top: yPadding } } : {}), readOnly, renderLineHighlight, - lineDecorationsWidth: 0, - lineNumbersMinChars: 2, + // Same conditional as `updateModelAndOptions`: created correct rather than + // created wide and narrowed a tick later, which a caller hiding the gutter + // would see as a flash of indent. + lineDecorationsWidth: hideLineNumbers ? 0 : LINE_DECORATIONS_WIDTH, + lineNumbersMinChars: hideLineNumbers ? 0 : lineNumbersMinChars, fontSize: fontSize, quickSuggestions: disableSuggestions ? { other: false, comments: false, strings: false } diff --git a/frontend/src/lib/components/common/fileInput/FileInput.svelte b/frontend/src/lib/components/common/fileInput/FileInput.svelte index 11f7a0caa9..bde73b8b69 100644 --- a/frontend/src/lib/components/common/fileInput/FileInput.svelte +++ b/frontend/src/lib/components/common/fileInput/FileInput.svelte @@ -201,6 +201,13 @@ } } + /** Open the file chooser without the dropzone being clicked, for a caller whose + * affordance is a button elsewhere. The component is still what reads and filters + * the files, so the two paths cannot drift. */ + export function openPicker() { + input?.click() + } + export function clearFiles() { files = undefined dispatchChange() diff --git a/frontend/src/lib/components/common/index.ts b/frontend/src/lib/components/common/index.ts index 531036c3de..bb0268b39b 100644 --- a/frontend/src/lib/components/common/index.ts +++ b/frontend/src/lib/components/common/index.ts @@ -20,6 +20,7 @@ export { default as TabFade } from './tabs/TabFade.svelte' export { default as Tabs } from './tabs/Tabs.svelte' export { default as Breadcrumb } from './breadcrumb/Breadcrumb.svelte' export { default as FileInput } from './fileInput/FileInput.svelte' +export { default as ListRow } from './listRow/ListRow.svelte' export { default as RadioCard } from './radioCard/RadioCard.svelte' export { default as Section } from '../Section.svelte' export { default as Url } from './Url.svelte' diff --git a/frontend/src/lib/components/common/listRow/ListRow.svelte b/frontend/src/lib/components/common/listRow/ListRow.svelte new file mode 100644 index 0000000000..dfa56d5af3 --- /dev/null +++ b/frontend/src/lib/components/common/listRow/ListRow.svelte @@ -0,0 +1,142 @@ + + + +{#snippet body()} +
    + {#if icon} +
    {@render icon()}
    + {/if} +
    +
    + {@render title()} +
    + {#if subtitle} + + + {@render subtitle()} + + {/if} +
    +
    +{/snippet} + +{#if trailing} + +
    + {#if onClick} + + + {:else} + +
    {@render body()}
    + {/if} + {@render trailing()} +
    +{:else if !onClick} + + +
    + {@render body()} +
    +{:else} + +{/if} diff --git a/frontend/src/lib/components/common/listRow/listHighlight.svelte.ts b/frontend/src/lib/components/common/listRow/listHighlight.svelte.ts new file mode 100644 index 0000000000..4da6f18f32 --- /dev/null +++ b/frontend/src/lib/components/common/listRow/listHighlight.svelte.ts @@ -0,0 +1,84 @@ +import { untrack } from 'svelte' + +/** + * The highlighted row of a searchable list: the one the arrow keys move and Enter + * activates, rendered by passing `highlighted` to `ListRow`. + * + * Pairs with a search field above the list — the arrows and Enter are answered while + * focus stays in it, so a query and a choice are one uninterrupted sequence. + */ +export function useListHighlight(opts: { + /** How many rows the list holds right now. */ + count: () => number + /** The DOM id of the row at this index — the same `id` given to its `ListRow`. */ + rowId: (index: number) => string + /** Where the highlight belongs when the list changes underneath it: the top hit while + * a search is on, and typically -1 (nothing lit) when it is not. */ + restingIndex: () => number + /** Open the row at this index. */ + onActivate: (index: number) => void + /** Ids of the elements whose Enter also activates the highlighted row — the search + * field. A focused row activates itself, so it is not one of these. */ + activateEnterFrom?: string[] +}) { + let index = $state(-1) + // Scrolling rows under a resting pointer makes the browser fire `mouseenter` on each + // one, which would drag the highlight back under the cursor as the arrow keys move it. + // Only a real pointer move hands the highlight back to the mouse. + let pointerOwns = $state(true) + + // Filtering reshuffles the rows under the highlight, so it goes back where the caller + // says it belongs rather than staying on a position that now means another row. + $effect(() => { + opts.count() + const resting = opts.restingIndex() + untrack(() => (index = resting)) + }) + + function move(delta: number) { + const count = opts.count() + if (count === 0) return + pointerOwns = false + // Rows are tabbable, so focus can sit on one. Enter then activates whatever is + // focused, which has to stay the highlighted row — so any row counts, not just + // the lit one. Tab from the search field lands on the first row while the + // highlight rests on the best match, and testing only the lit row would leave + // focus behind and activate the wrong one. + const focusedId = document.activeElement?.id + const rowWasFocused = + !!focusedId && Array.from({ length: count }, (_, i) => opts.rowId(i)).includes(focusedId) + index = index < 0 ? (delta > 0 ? 0 : count - 1) : (index + delta + count) % count + const row = document.getElementById(opts.rowId(index)) + row?.scrollIntoView({ block: 'nearest' }) + if (rowWasFocused) row?.focus() + } + + return { + get index() { + return index + }, + /** Wire to each row's `onMouseEnter`. */ + hovered(i: number) { + if (pointerOwns) index = i + }, + /** Wire to the list container's `onpointermove`. */ + pointerMoved() { + pointerOwns = true + }, + /** Wire to the container that holds the search field and the rows, so the keys are + * answered whichever of the two has focus. */ + onKeydown(e: KeyboardEvent) { + if (e.key === 'ArrowDown' || e.key === 'ArrowUp') { + e.preventDefault() + move(e.key === 'ArrowDown' ? 1 : -1) + } else if ( + e.key === 'Enter' && + opts.activateEnterFrom?.includes((e.target as HTMLElement)?.id) && + index >= 0 + ) { + e.preventDefault() + opts.onActivate(index) + } + } + } +} diff --git a/frontend/src/lib/components/common/modal/Modal2.svelte b/frontend/src/lib/components/common/modal/Modal2.svelte index f33b1dec04..b9e57740b9 100644 --- a/frontend/src/lib/components/common/modal/Modal2.svelte +++ b/frontend/src/lib/components/common/modal/Modal2.svelte @@ -26,6 +26,10 @@ * and clicks "outside" the child would otherwise propagate * here and close the underlying modal. */ closeOnOutsideClick?: boolean + /** Close on Escape. Default true. Every open modal listens on the + * window, so a stacked pair would both close on one press; set it + * false on the underlying modal while its child is up. */ + closeOnEscape?: boolean /** Wider side padding and a lighter title, for a dialog whose body is a form rather * than a list. Opt-in: every other Modal2 keeps the padding and heading it had. */ formStyling?: boolean @@ -46,6 +50,7 @@ fixedHeight = 'md', contentClasses = '', closeOnOutsideClick = true, + closeOnEscape = true, formStyling = false, headerLeft, headerRight, @@ -80,7 +85,7 @@ } function handleKeyDown(event: KeyboardEvent) { - if (!isOpen) return + if (!isOpen || !closeOnEscape) return if (event.key === 'Escape') { event.preventDefault() event.stopPropagation() diff --git a/frontend/src/lib/components/common/modal/PagedContent.svelte b/frontend/src/lib/components/common/modal/PagedContent.svelte index 430a53e66f..674c147c52 100644 --- a/frontend/src/lib/components/common/modal/PagedContent.svelte +++ b/frontend/src/lib/components/common/modal/PagedContent.svelte @@ -26,6 +26,9 @@ * around this owns it, and a page component that swallowed it would stop the dialog from * closing. Without this prop the pages are still navigable, just not from the keyboard — * the caller owns `current` either way. + * + * A host that stays mounted while hidden must withhold it while hidden: the arrows are + * answered at `window`, so a parked instance would take the key off the visible one. */ onNavigate?: (key: string) => void /** @@ -80,6 +83,9 @@ function onKeydown(event: KeyboardEvent) { if (!onNavigate || !listening() || event.metaKey || event.ctrlKey || event.altKey) return + // A control on the page that already answered the key keeps it: the arrows move focus + // inside a toggle group, a menu, a slider, and those handlers run before this one. + if (event.defaultPrevented) return if (!ownsKeyboard(event.target)) return const step = event.key === 'ArrowRight' ? 1 : event.key === 'ArrowLeft' ? -1 : 0 if (step === 0) return diff --git a/frontend/src/lib/components/common/sidebar/SidebarNavigation.svelte b/frontend/src/lib/components/common/sidebar/SidebarNavigation.svelte index 260e368e2e..d18df4c655 100644 --- a/frontend/src/lib/components/common/sidebar/SidebarNavigation.svelte +++ b/frontend/src/lib/components/common/sidebar/SidebarNavigation.svelte @@ -2,6 +2,7 @@ import type { ComponentType } from 'svelte' import { twMerge } from 'tailwind-merge' import Button from '$lib/components/common/button/Button.svelte' + import { arrowTabNav } from '$lib/attachments/arrowTabNav' import EEOnly from '$lib/components/EEOnly.svelte' import { enterpriseLicense } from '$lib/stores' @@ -32,7 +33,10 @@ let { groups, selectedId, onNavigate, class: className = '' }: Props = $props() -
    + +
    {#each groups as group (group.title)}
    {#if group.title} diff --git a/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte b/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte index f610dd5ff4..699098c116 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte @@ -35,8 +35,9 @@ import ChatQuickActions from './ChatQuickActions.svelte' import ContextUsageIndicator from './ContextUsageIndicator.svelte' import AIChatModelSettings from './AIChatModelSettings.svelte' - import McpConnections from './McpConnections.svelte' - import SkillsPicker from './SkillsPicker.svelte' + import AssistantSettingsModal from './AssistantSettingsModal.svelte' + import { SkillsMenu } from './skills/skillsMenu.svelte' + import { McpMenu } from '$lib/components/mcp/mcpMenu.svelte' import ChatMode from './ChatMode.svelte' import DatatableCreationPolicy from './DatatableCreationPolicy.svelte' import Tooltip from '$lib/components/meltComponents/Tooltip.svelte' @@ -207,8 +208,11 @@ } = $props() let aiChatInput: AIChatInput | undefined = $state() - let mcpConnections: McpConnections | undefined = $state() - let skillsPicker: SkillsPicker | undefined = $state() + let assistantSettings: AssistantSettingsModal | undefined = $state() + // The "+" menu's skill and MCP rows: enough state to check and flip one, with + // everything else about them behind the assistant settings modal. + const skillsMenu = new SkillsMenu(aiChatManager, () => assistantSettings?.open('skills')) + const mcpMenu = new McpMenu(aiChatManager, () => assistantSettings?.open('mcp')) let plusMenuOpen = $state(false) let editingMessageIndex = $state(null) @@ -959,8 +963,8 @@ the panel, or the Escape-to-stop focus check would wrongly reject them. --> const closeMenu = () => (plusMenuOpen = false) const inGlobal = aiChatManager.mode === AIMode.GLOBAL const [skillItems, mcpItems] = await Promise.all([ - inGlobal ? skillsPicker?.menuItems(closeMenu) : undefined, - inGlobal ? mcpConnections?.menuItems(closeMenu) : undefined + inGlobal ? skillsMenu.items(closeMenu) : undefined, + inGlobal ? mcpMenu.items(closeMenu) : undefined ]) return [ { @@ -1143,10 +1147,12 @@ the panel, or the Escape-to-stop focus check would wrongly reject them. --> {/if} - + + {#if aiChatManager.mode === AIMode.GLOBAL} - - + {/if} {#if aiChatManager.mode === AIMode.APP && appContext && (appContext.inspectorElement || appContext.codeSelection)} diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index fb54204fe9..43df13f78c 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -2123,7 +2123,10 @@ export class AIChatManager { // pipeline surface when a /pipeline editor has registered helpers. Centralized // so changeMode, refreshGlobalSkills, and setPipelineHelpers stay consistent — // each rebuild would otherwise drop the pipeline augmentation the others added. - private configureGlobalMode = () => { + // + // Public because it is purely local, unlike `changeMode(GLOBAL)`, which also + // fires the three network refreshes. + configureGlobalMode = () => { const systemMessage = prepareGlobalSystemMessage(getCustomPromptParts(AIMode.GLOBAL), { previewTools: this.isSessionChat, user: this.globalIdentity, diff --git a/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte b/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte index 123768d3f3..9dd94fa4ea 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte @@ -33,6 +33,13 @@ type ReasoningProviderModel } from '../reasoningRegistry' + let { + /** Whether this dropdown carries the custom-prompt entries. Off where the surface + * has an assistant settings modal — its Instructions section owns them there, and + * two ways in would drift. The home composer has no such modal, so it keeps them. */ + promptSettings = true + }: { promptSettings?: boolean } = $props() + const aiChatManager = getAiChatManager() const AI_SETTINGS_HREF = `${base}/workspace_settings?tab=ai` @@ -335,7 +342,9 @@ class="bg-surface-tertiary dark:border w-64 origin-top-right rounded-lg shadow-lg focus:outline-none py-1 text-xs" > - + {#if promptSettings} + + {/if}
    Model
    @@ -409,21 +418,24 @@ {/snippet} - + +{#if promptSettings} + +{/if} diff --git a/frontend/src/lib/components/home/TutorialBanner.svelte b/frontend/src/lib/components/home/TutorialBanner.svelte deleted file mode 100644 index ef7b30c27e..0000000000 --- a/frontend/src/lib/components/home/TutorialBanner.svelte +++ /dev/null @@ -1,178 +0,0 @@ - - -{#if !isDismissed} - -
    - - {#if hasCompletedAny} - New tutorial available! - {:else} - First time? - {/if} - - - -
    -{/if} diff --git a/frontend/src/lib/components/home/TutorialButton.svelte b/frontend/src/lib/components/home/TutorialButton.svelte deleted file mode 100644 index 6a31ae9569..0000000000 --- a/frontend/src/lib/components/home/TutorialButton.svelte +++ /dev/null @@ -1,124 +0,0 @@ - - - - diff --git a/frontend/src/lib/components/home/WorkspaceEmptyState.svelte b/frontend/src/lib/components/home/WorkspaceEmptyState.svelte new file mode 100644 index 0000000000..fc36cc334b --- /dev/null +++ b/frontend/src/lib/components/home/WorkspaceEmptyState.svelte @@ -0,0 +1,146 @@ + + +
    + {#each rowOpacities as opacity, i (i)} + + {/each} + + +
    + {#if archivedOnly} + + + Everything in this workspace is archived. + . + + {:else} + Your scripts, flows and apps will show up here. + {/if} + {#if canCreate} + + {#if !$disableHubStore} + + + e.detail && logFeatureUsage('home', 'template_picker_open', { key: 'empty_state' })} + > + {#snippet trigger()}Start from a template{/snippet} + {#snippet content({ close })} + { + close() + onPick(project) + }} + /> + {/snippet} + + or + {/if} + + {#snippet trigger()} + + . + {/snippet} + + {/if} +
    +
    diff --git a/frontend/src/lib/components/sidebar/OperatorMenu.svelte b/frontend/src/lib/components/sidebar/OperatorMenu.svelte index 0914068607..fa59d730df 100644 --- a/frontend/src/lib/components/sidebar/OperatorMenu.svelte +++ b/frontend/src/lib/components/sidebar/OperatorMenu.svelte @@ -12,10 +12,11 @@ Building, Calendar, ServerCog, - GraduationCap, - Table2 + Table2, + GraduationCap } from 'lucide-svelte' import { base } from '$lib/base' + import { TOUR_PARAM, TOUR_PARAM_VALUE } from '$lib/components/tutorials/operatorTour' import MultiplayerMenu from './MultiplayerMenu.svelte' import { Plus } from 'lucide-svelte' @@ -25,9 +26,7 @@ superadmin, usedTriggerKinds, userWorkspaces, - workspaceStore, - tutorialsToDo, - skippedAll + workspaceStore } from '$lib/stores' import { twMerge } from 'tailwind-merge' import { USER_SETTINGS_HASH } from './settings' @@ -56,22 +55,10 @@ [ { label: 'Home', id: 'home', href: `${base}/`, icon: Home }, { label: 'Runs', id: 'runs', href: `${base}/runs`, icon: Play }, - { label: 'Schedules', id: 'schedules', href: `${base}/schedules`, icon: Calendar }, - // Add Tutorials to main menu only if not all completed and not skipped - ...($tutorialsToDo.length > 0 && !$skippedAll - ? [ - { - label: 'Tutorials', - id: 'tutorials', - href: `${base}/tutorials`, - icon: GraduationCap - } - ] - : []) + { label: 'Schedules', id: 'schedules', href: `${base}/schedules`, icon: Calendar } ].filter( (link) => link.id === 'home' || - link.id === 'tutorials' || ($userWorkspaces && $workspaceStore && $userWorkspaces.find((_) => _.id === $workspaceStore)?.operator_settings?.[link.id] === @@ -243,6 +230,21 @@ Account settings + + + + Take the tour +
    diff --git a/frontend/src/lib/components/sidebar/SettingsMenu.svelte b/frontend/src/lib/components/sidebar/SettingsMenu.svelte index fbc07a775f..b7a3a363bb 100644 --- a/frontend/src/lib/components/sidebar/SettingsMenu.svelte +++ b/frontend/src/lib/components/sidebar/SettingsMenu.svelte @@ -12,7 +12,6 @@ Building, Moon, Sun, - GraduationCap, BookOpen, Github, Newspaper, @@ -120,7 +119,6 @@ } const helpItems: Item[] = [ - { displayName: 'Tutorials', icon: GraduationCap, href: `${base}/tutorials` }, { displayName: 'Docs', icon: BookOpen, diff --git a/frontend/src/lib/components/sidebar/SidebarContent.svelte b/frontend/src/lib/components/sidebar/SidebarContent.svelte index fb1bb9ee17..7fa161dd2d 100644 --- a/frontend/src/lib/components/sidebar/SidebarContent.svelte +++ b/frontend/src/lib/components/sidebar/SidebarContent.svelte @@ -8,12 +8,9 @@ workspaceStore, isCriticalAlertsUIOpen, enterpriseLicense, - devopsRole, - tutorialsToDo, - skippedAll + devopsRole } from '$lib/stores' import { isForkOwner } from '$lib/utils/workspaceHierarchy' - import { syncTutorialsTodos } from '$lib/tutorialUtils' import { SIDEBAR_SHOW_SCHEDULES } from '$lib/consts' import { BookOpen, @@ -26,7 +23,6 @@ FolderCog, FolderOpen, Github, - GraduationCap, HelpCircle, Home, LogOut, @@ -51,7 +47,6 @@ import DiscordIcon from '../icons/brands/Discord.svelte' import ConfirmationModal from '../common/confirmationModal/ConfirmationModal.svelte' import { twMerge } from 'tailwind-merge' - import { onMount } from 'svelte' import { base } from '$lib/base' import { page } from '$app/state' import SideBarNotification from './SideBarNotification.svelte' @@ -116,11 +111,6 @@ 'boolean' ) - onMount(async () => { - // Sync tutorial progress on mount - await syncTutorialsTodos() - }) - function openChangelogs() { markChangelogsOpened() hasNewChangelogs = false @@ -131,14 +121,6 @@ label: 'Help', icon: HelpCircle, subItems: [ - { - label: 'Tutorials', - href: `${base}/tutorials`, - icon: GraduationCap, - aiId: 'sidebar-menu-link-tutorials', - aiDescription: 'Button to navigate to tutorials', - external: false - }, { label: 'Docs', href: 'https://www.windmill.dev/docs/intro/', @@ -269,19 +251,7 @@ disabled: $userStore?.operator, aiId: 'sidebar-menu-link-groups', aiDescription: 'Button to navigate to groups' - }, - // Add Tutorials to main menu only if not all completed and not skipped - ...($tutorialsToDo.length > 0 && !$skippedAll - ? [ - { - label: 'Tutorials', - href: `${base}/tutorials`, - icon: GraduationCap, - aiId: 'sidebar-menu-link-tutorials-main', - aiDescription: 'Button to navigate to tutorials' - } - ] - : []) + } ].filter((l) => !excludeMainLabels.includes(l.label)) ) let defaultExtraTriggerLinks = $derived([ diff --git a/frontend/src/lib/components/tutorials/FlowBuilderLiveTutorial.svelte b/frontend/src/lib/components/tutorials/FlowBuilderLiveTutorial.svelte deleted file mode 100644 index 343deb917c..0000000000 --- a/frontend/src/lib/components/tutorials/FlowBuilderLiveTutorial.svelte +++ /dev/null @@ -1,754 +0,0 @@ - - - { - const steps: DriveStep[] = [ - { - popover: { - title: 'Build your first flow', - description: - "Let's create a temperature converter that validates input and converts Celsius to Fahrenheit.", - onNextClick: async () => { - const emptyFlow: Flow = { - summary: '', - description: '', - value: { modules: [] }, - schema: flowJson.schema, - path: '', - edited_at: '', - edited_by: '', - archived: false, - extra_perms: {} - } - await initFlow(emptyFlow, flowStore as StateStore, flowStateStore) - - driver.moveNext() - } - } - }, - { - element: '#flow-editor-virtual-Input', - onHighlighted: async () => { - step2Complete = false - - await wait(DELAY_MEDIUM) - triggerPointerDown('#flow-editor-virtual-Input') - await wait(DELAY_SHORT) - selectionManager.selectId('Input') - await wait(200) - - const overlay = getDriverOverlay() - if (overlay) { - overlay.style.width = '50%' - overlay.style.right = 'auto' - overlay.style.left = '0' - } - - const celsiusInput = document.querySelector( - 'input[type="number"][placeholder=""]' - ) as HTMLInputElement - if (celsiusInput) { - celsiusInput.value = '' - celsiusInput.dispatchEvent(new Event('input', { bubbles: true })) - await wait(DELAY_MEDIUM) - - celsiusInput.value = '2' - celsiusInput.dispatchEvent(new Event('input', { bubbles: true })) - await wait(400) - - celsiusInput.value = '25' - celsiusInput.dispatchEvent(new Event('input', { bubbles: true })) - - step2Complete = true - } - }, - popover: { - title: 'Set the input', - description: 'Every flow starts with input. Here we define a temperature in Celsius.', - side: 'bottom', - align: 'start', - onNextClick: () => { - if (!step2Complete) { - sendUserToast('Please wait for the input to be filled...', false, [], undefined, 3000) - return - } - driver.moveNext() - } - } - }, - { - element: '#flow-editor-add-step-0', - onHighlighted: async () => { - step3Complete = false - - // Animate cursor to the add step button - const button = document.querySelector('#flow-editor-add-step-0') as HTMLElement - if (button) { - const fakeCursor1 = await createFakeCursorWithStart(null, button, 1.5) - await wait(DELAY_SHORT) - button.click() - fakeCursor1.remove() - } - - const overlay = getDriverOverlay() - if (overlay) { - overlay.style.display = 'none' - } - - await wait(DELAY_LONG) - - const spans = Array.from(document.querySelectorAll('span')) - const bunSpan = spans.find((span) => - span.textContent?.includes('TypeScript (Bun)') - ) as HTMLElement - - if (bunSpan) { - // Animate cursor from add step button to TypeScript (Bun) span - const fakeCursor2 = await createFakeCursorWithStart(button, bunSpan, 1.5) - await wait(DELAY_MEDIUM) - fakeCursor2.remove() - - // Automatically trigger next step after cursor animation - await wait(DELAY_SHORT) - - // Add module with empty summary and empty content - const moduleData = flowJson.value.modules[0] - const module: FlowModule = { - id: moduleData.id, - summary: '', // Start with empty summary - value: moduleData.value - } - // Clear content after module creation if it's a rawscript - if ('content' in module.value) { - module.value = { ...module.value, content: '' } as typeof module.value - } - - await addModuleToFlow(module) - - await wait(700) - - // Restore overlay - const overlay = getDriverOverlay() - if (overlay) { - overlay.style.display = '' - } - - step3Complete = true - driver.moveNext() - } - }, - popover: { - title: 'Choose TypeScript', - description: 'Pick TypeScript (Bun) to write our validation script.', - side: 'top', - onNextClick: () => { - if (!step3Complete) { - sendUserToast( - 'Please wait for the script to be created...', - false, - [], - undefined, - 3000 - ) - return - } - driver.moveNext() - }, - onPrevClick: () => { - sendUserToast('Previous is not available for this step', true, [], undefined, 3000) - } - } - }, - { - element: '#a', - onHighlighted: async () => { - // Reset the flag when step starts - step4Complete = false - - selectionManager.selectId('a') - await wait(DELAY_LONG) - - const overlay = getDriverOverlay() - if (overlay) { - overlay.style.width = '50%' - overlay.style.right = 'auto' - overlay.style.left = '0' - } - - // First, type the summary - await wait(DELAY_MEDIUM) - const summaryInput = document.querySelector( - 'input[placeholder="Summary"]' - ) as HTMLInputElement - if (summaryInput) { - const summaryText = 'Validate temperature input' - await typeText(summaryInput, summaryText) - updateModuleSummary('a', summaryText) - await wait(DELAY_LONG) - } - - // Then, type the code - let editorState = get(currentEditor) - let attempts = 0 - while (attempts < 20) { - if (editorState && editorState.type === 'script' && editorState.stepId === 'a') { - break - } - await wait(100) - editorState = get(currentEditor) - attempts++ - } - - if (editorState && editorState.type === 'script') { - const editor = editorState.editor - const moduleA = flowJson.value.modules.find((m) => m.id === 'a') - const codeToType = - moduleA?.value && 'content' in moduleA.value ? moduleA.value.content : '' - - if (codeToType) { - editor.setCode('', true) - await wait(200) - - let currentText = '' - for (let i = 0; i < codeToType.length; i++) { - const char = codeToType[i] - currentText += char - editor.setCode(currentText, true) - const delay = char === '\n' ? DELAY_CODE_NEWLINE : DELAY_CODE_CHAR - await wait(delay) - } - - // Update the flow store with the typed code - const moduleIndex = flowStore.val.value.modules.findIndex((m) => m.id === 'a') - if ( - moduleIndex !== -1 && - 'content' in flowStore.val.value.modules[moduleIndex].value - ) { - flowStore.val.value.modules[moduleIndex].value = { - ...flowStore.val.value.modules[moduleIndex].value, - content: codeToType - } - flowStore.val = { ...flowStore.val } - } - - // Press Enter after finishing typing - await wait(DELAY_MEDIUM) - const model = editor.getModel() - if (model && 'setValue' in model) { - model.setValue(currentText + '\n') - } - - // Mark step 4 as complete - step4Complete = true - } - } - }, - popover: { - title: 'Add validation logic', - description: 'Watch as we write code to validate the temperature input.', - side: 'bottom', - onNextClick: () => { - // Only proceed if code writing is complete - if (!step4Complete) { - sendUserToast( - 'Please wait for the code to finish typing...', - false, - [], - undefined, - 3000 - ) - return - } - - const driverOverlay = getDriverOverlay() - if (driverOverlay) { - driverOverlay.style.display = 'none' - } - - const customOverlay = document.createElement('div') - customOverlay.className = 'tutorial-custom-overlay' - customOverlay.style.cssText = ` - position: fixed; - top: 0; - left: 0; - width: 100%; - height: 100%; - background-color: rgba(0, 0, 0, 0.5); - z-index: 9999; - pointer-events: none; - clip-path: polygon( - 0 0, 100% 0, 100% 50%, 50% 50%, 50% 100%, 0 100% - ); - ` - document.body.appendChild(customOverlay) - - driver.moveNext() - }, - onPrevClick: () => { - sendUserToast('Previous is not available for this step', true, [], undefined, 3000) - } - } - }, - { - onHighlighted: async () => { - step5Complete = false - - // Create a single cursor that will move continuously - const fakeCursor = document.createElement('div') - fakeCursor.style.cssText = ` - position: fixed; - width: 20px; - height: 20px; - border-radius: 50%; - background-color: rgba(59, 130, 246, 0.8); - border: 2px solid white; - pointer-events: none; - z-index: 10000; - transition: all 1.5s ease-in-out; - ` - document.body.appendChild(fakeCursor) - - // Step 1: Move to and click plug button - document.querySelector('#flow-editor-plug')?.parentElement?.classList.remove('opacity-0') - await wait(DELAY_SHORT) - const plugButton = document.querySelector('#flow-editor-plug') as HTMLElement - if (plugButton) { - const plugRect = plugButton.getBoundingClientRect() - // Start from off-screen left - fakeCursor.style.left = `${plugRect.left - 100}px` - fakeCursor.style.top = `${plugRect.top + plugRect.height / 2}px` - await wait(DELAY_SHORT) - // Move to plug button - fakeCursor.style.left = `${plugRect.left + plugRect.width / 2}px` - fakeCursor.style.top = `${plugRect.top + plugRect.height / 2}px` - await wait(DELAY_ANIMATION) - await wait(DELAY_MEDIUM) - clickButtonBySelector('#flow-editor-plug') - } - - await wait(DELAY_MEDIUM) - - // Step 2: Move to and click flow_input.celsius - const targetButton = document.querySelector( - 'button[title="flow_input.celsius"]' - ) as HTMLElement - if (targetButton) { - await moveCursorToElement(fakeCursor, targetButton, DELAY_ANIMATION_LONG) - await wait(DELAY_MEDIUM) - const clickEvent = new MouseEvent('click', { - bubbles: true, - cancelable: true, - view: window - }) - targetButton.dispatchEvent(clickEvent) - } - - await wait(DELAY_LONG) - - // Step 3: Move to and click Test this step tab - const testTabButton = findButtonByText('Test this step', ['border-b-2', 'cursor-pointer']) - - if (testTabButton) { - await moveCursorToElement(fakeCursor, testTabButton, DELAY_ANIMATION) - await wait(DELAY_SHORT) - testTabButton.click() - } - - await wait(DELAY_LONG) - - // Step 4: Move to and click Run button - const testActionButton = findButtonByText('Run', ['bg-surface-accent-primary', 'w-full']) - - if (testActionButton) { - await moveCursorToElement(fakeCursor, testActionButton, DELAY_ANIMATION) - await wait(DELAY_MEDIUM) - testActionButton.click() - await wait(DELAY_MEDIUM) - } - - // Remove cursor at the end - fakeCursor.remove() - - step5Complete = true - }, - popover: { - title: 'Wire it up and test', - description: 'Connect the input, then run a quick test to verify the validation works.', - onNextClick: async () => { - if (!step5Complete) { - sendUserToast('Please wait for the test to complete...', false, [], undefined, 3000) - return - } - cleanupCustomOverlay() - driver.moveNext() - }, - onPrevClick: () => { - sendUserToast('Previous is not available for this step', true, [], undefined, 3000) - } - } - }, - { - onHighlighted: async () => { - step6Complete = false - - // First, add modules b and c with empty summaries - const modulesToAdd = [flowJson.value.modules[1], flowJson.value.modules[2]] - for (let i = 0; i < modulesToAdd.length; i++) { - await new Promise((resolve) => setTimeout(resolve, i === 0 ? 0 : 700)) - - const moduleData = modulesToAdd[i] - const module: FlowModule = { - id: moduleData.id, - summary: '', // Start with empty summary - value: moduleData.value - } - - await addModuleToFlow(module) - } - - await wait(700) - - // Create a single cursor for continuous movement - const fakeCursor = document.createElement('div') - fakeCursor.style.cssText = ` - position: fixed; - width: 20px; - height: 20px; - border-radius: 50%; - background-color: rgba(59, 130, 246, 0.8); - border: 2px solid white; - pointer-events: none; - z-index: 10000; - transition: all 1.5s ease-in-out; - ` - document.body.appendChild(fakeCursor) - - // Step 1: Click on script 'b' - await wait(DELAY_MEDIUM) - const scriptB = document.querySelector('#b') as HTMLElement - if (scriptB) { - const bRect = scriptB.getBoundingClientRect() - // Start from off-screen - fakeCursor.style.left = `${bRect.left - 100}px` - fakeCursor.style.top = `${bRect.top + bRect.height / 2}px` - await wait(DELAY_SHORT) - // Move to script b - fakeCursor.style.left = `${bRect.left + bRect.width / 2}px` - fakeCursor.style.top = `${bRect.top + bRect.height / 2}px` - await wait(DELAY_ANIMATION) - await wait(DELAY_MEDIUM) - selectionManager.selectId('b') - } - - await wait(DELAY_LONG) - - // Type summary for script 'b' - const summaryInputB = document.querySelector( - 'input[placeholder="Summary"]' - ) as HTMLInputElement - if (summaryInputB) { - const summaryTextB = 'Convert to Fahrenheit' - await typeText(summaryInputB, summaryTextB) - updateModuleSummary('b', summaryTextB) - await wait(DELAY_LONG) - } - - // Step 2: Move to and click on script 'c' - const scriptC = document.querySelector('#c') as HTMLElement - if (scriptC) { - await moveCursorToElement(fakeCursor, scriptC, DELAY_ANIMATION) - await wait(DELAY_SHORT) - selectionManager.selectId('c') - } - - await wait(DELAY_LONG) - - // Type summary for script 'c' - const summaryInputC = document.querySelector( - 'input[placeholder="Summary"]' - ) as HTMLInputElement - if (summaryInputC) { - const summaryTextC = 'Categorize temperature' - await typeText(summaryInputC, summaryTextC) - updateModuleSummary('c', summaryTextC) - await wait(DELAY_LONG) - } - - // Move cursor to Test Flow button - const testFlowButton = document.querySelector('#flow-editor-test-flow') as HTMLElement - if (testFlowButton) { - await moveCursorToElement(fakeCursor, testFlowButton, DELAY_ANIMATION) - await wait(DELAY_MEDIUM) - } - - // Remove cursor at the end - fakeCursor.remove() - - step6Complete = true - }, - popover: { - title: 'Add the final steps', - description: 'Two more scripts to convert and categorize the temperature.', - onNextClick: () => { - if (!step6Complete) { - sendUserToast( - 'Please wait for the summaries to be added...', - false, - [], - undefined, - 3000 - ) - return - } - - // Reset the driver.js overlay to full screen - const driverOverlay = getDriverOverlay() - if (driverOverlay) { - driverOverlay.style.display = '' - driverOverlay.style.width = '' - driverOverlay.style.right = '' - driverOverlay.style.left = '' - } - driver.moveNext() - }, - onPrevClick: () => { - sendUserToast('Previous is not available for this step', true, [], undefined, 3000) - } - } - }, - { - element: '#flow-editor-test-flow', - popover: { - title: 'Ready to test!', - description: - 'Run the complete flow and see your temperature converter in action.

    💡 Want to learn more? Access more tutorials from the Tutorials page in the main menu or in the Help submenu.

    ', - onNextClick: () => { - updateProgress(index) - driver.destroy() - }, - onPrevClick: () => { - sendUserToast('Previous is not available for this step', true, [], undefined, 3000) - } - } - } - ] - - return steps - }} -/> diff --git a/frontend/src/lib/components/tutorials/OperatorTour.svelte b/frontend/src/lib/components/tutorials/OperatorTour.svelte new file mode 100644 index 0000000000..cc7f4ab4a5 --- /dev/null +++ b/frontend/src/lib/components/tutorials/OperatorTour.svelte @@ -0,0 +1,81 @@ + + + { + const steps: DriveStep[] = [ + { + popover: { + title: 'Welcome to Windmill! 🎉', + description: + "Let's take a quick tour! We'll show you the three main tools you can use: Scripts, Flows, and Apps." + } + }, + { + popover: { + title: 'Scripts - Run automated tasks', + description: + 'Script Example

    Scripts are ready-to-use tasks that do things automatically for you.

    You can run scripts whenever you need them - like generating a report, sending notifications, or processing data.

    ' + }, + element: '[data-value="script"]' + }, + { + popover: { + title: 'Flows - Run step-by-step processes', + description: + 'Flow

    Flows are processes that run multiple tasks in order, one after another.

    You can start a flow and watch it complete each step automatically - perfect for tasks that have multiple stages.

    ' + }, + element: '[data-value="flow"]' + }, + { + popover: { + title: 'Apps - Use custom tools', + description: + 'App

    Apps are easy-to-use tools with buttons, forms, and displays built just for your team.

    You can open an app to work with your data, fill out forms, or trigger tasks - no technical knowledge needed!

    ' + }, + element: '[data-value="app"]' + }, + { + popover: { + title: 'Finally, the Menu section', + description: + 'Explore available tabs where you can access your history of runs, your scheduled scripts, and your workspaces.

    💡 Want to see this again? Pick Take the tour from that same menu.

    ', + onNextClick: async () => { + // The step points into the menu, so it has to be open before the popover + // lands on it — and open is also where the entry to re-run the tour is. + const menuButton = document.querySelector('[role="menuitem"]') as HTMLElement | null + menuButton?.click() + await wait(MENU_OPEN_DELAY_MS) + driver.destroy() + } + }, + element: '[role="menuitem"]' + } + ] + + return steps + }} +/> diff --git a/frontend/src/lib/components/tutorials/RunsTutorial.svelte b/frontend/src/lib/components/tutorials/RunsTutorial.svelte deleted file mode 100644 index 0b19ac5f0e..0000000000 --- a/frontend/src/lib/components/tutorials/RunsTutorial.svelte +++ /dev/null @@ -1,510 +0,0 @@ - - - { - return getTutorialSteps(driver) - }} -/> diff --git a/frontend/src/lib/components/tutorials/SkipTutorials.svelte b/frontend/src/lib/components/tutorials/SkipTutorials.svelte deleted file mode 100644 index 6fd6ba254b..0000000000 --- a/frontend/src/lib/components/tutorials/SkipTutorials.svelte +++ /dev/null @@ -1,32 +0,0 @@ - - -
    - - -
    diff --git a/frontend/src/lib/components/tutorials/TroubleshootFlowTutorial.svelte b/frontend/src/lib/components/tutorials/TroubleshootFlowTutorial.svelte deleted file mode 100644 index 4fa9909e02..0000000000 --- a/frontend/src/lib/components/tutorials/TroubleshootFlowTutorial.svelte +++ /dev/null @@ -1,441 +0,0 @@ - - - { - const steps: DriveStep[] = [ - { - popover: { - title: '🛠️ Troubleshoot a broken flow', - description: - 'We created a flow that is a temperature converter that validates input and converts Celsius to Fahrenheit. For this tutorial, our flow is intentionally broken.', - onNextClick: () => { - driver.moveNext() - } - } - }, - { - element: SELECTORS.testFlowButton, - onHighlighted: async () => { - stepComplete[1] = false - await wait(DELAY_SHORT) - stepComplete[1] = true - }, - popover: { - title: 'Test our flow', - description: - 'Let\'s run it so you can see what needs to be fixed.', - side: 'bottom', - onNextClick: async () => { - if (!checkStepComplete(1)) return - - // Click the Test Flow button to open the drawer - const testFlowButton = document.querySelector(SELECTORS.testFlowButton) as HTMLElement - if (testFlowButton) { - testFlowButton.click() - await wait(DELAY_LONG) - } - - driver.moveNext() - } - } - }, - { - element: SELECTORS.testFlowDrawer, - onHighlighted: async () => { - stepComplete[2] = false - await wait(DELAY_SHORT) - stepComplete[2] = true - }, - popover: { - title: 'Run the flow', - description: - 'Click "Next" to execute the flow. We\'ll use the results to troubleshoot the error.', - side: 'left', - onNextClick: async () => { - if (!checkStepComplete(2)) return - - // Click the Test button to execute the flow - const testButton = document.querySelector(SELECTORS.testFlowDrawer) as HTMLElement - if (testButton) { - testButton.click() - } - - await wait(DELAY_LONG) - driver.moveNext() - } - } - }, - { - element: '.border.rounded-md.shadow.p-2', - onHighlighted: async () => { - stepComplete[3] = false - await wait(DELAY_SHORT) - stepComplete[3] = true - }, - popover: { - title: 'Review the error', - description: - 'Our flow failed. Let\'s review the error and understand what happened.', - side: 'left', - onNextClick: () => { - if (!checkStepComplete(3)) return - driver.moveNext() - } - } - }, - { - element: '.border-b.flex.flex-row.whitespace-nowrap.scrollbar-hidden.mx-auto', - onHighlighted: async () => { - stepComplete[4] = false - await wait(DELAY_SHORT) - stepComplete[4] = true - }, - popover: { - title: 'Explore the tabs', - description: - 'Use these tabs to navigate between different views: Result, Logs, and Graph. We\'ll focus on the Graph tab to review the error.', - side: 'bottom', - onNextClick: () => { - if (!checkStepComplete(4)) return - driver.moveNext() - } - } - }, - { - element: '.grid.grid-cols-3.border.h-full', - onHighlighted: async () => { - stepComplete[5] = false - await wait(DELAY_SHORT) - - // Find the step 'b' button inside the drawer and click it with fake cursor - const flowPreviewContent = getElementBySelector(SELECTORS.flowPreviewContent) - if (flowPreviewContent) { - const stepButton = findButtonByText(flowPreviewContent, TEXT.convertToFahrenheit) - - if (stepButton) { - await animateFakeCursorClick(stepButton, 1.5, { usePointerEvents: true }) - await wait(DELAY_MEDIUM) - } - } - - stepComplete[5] = true - }, - popover: { - title: 'Inspect the flow graph', - description: - 'B step failed during the run. Let\'s take a closer look at its behavior.', - side: 'top', - onNextClick: () => { - if (!checkStepComplete(5)) return - driver.moveNext() - } - } - }, - { - element: '.rounded-md.grow.bg-surface-tertiary.text-xs.flex.flex-col.max-h-screen.gap-2.overflow-hidden.border', - onHighlighted: async () => { - stepComplete[6] = false - await wait(DELAY_SHORT) - stepComplete[6] = true - }, - popover: { - title: 'Error spotted!', - description: - 'We made a typo in the code. Let\'s fix it and run the flow again.', - side: 'left', - onNextClick: async () => { - if (!checkStepComplete(6)) return - - // Click the close button inside the drawer - const drawer = getElementBySelector(SELECTORS.flowPreviewContent) - if (drawer) { - const closeButton = findCloseButton(drawer) - - if (closeButton) { - await animateFakeCursorClick(closeButton, 1.5) - } - } - - await wait(DELAY_LONG) - driver.moveNext() - } - } - }, - { - element: SELECTORS.stepB, - onHighlighted: async () => { - stepComplete[7] = false - await wait(DELAY_SHORT) - - // Click on div id="b" to open the editor - const stepBDiv = getElementBySelector(SELECTORS.stepB) - if (stepBDiv) { - await animateFakeCursorClick(stepBDiv, 1.5) - await wait(DELAY_LONG) - } - - stepComplete[7] = true - }, - popover: { - title: 'Your turn now!', - description: - 'Fix the issue in the code, and run the flow again to confirm everything works.

    💡 Want to learn more? Access more tutorials from the Tutorials page in the main menu or in the Help submenu.

    ', - side: 'top', - onNextClick: () => { - if (!checkStepComplete(7)) return - updateProgress(index) - driver.destroy() - } - } - } - ] - - return steps - }} -/> diff --git a/frontend/src/lib/components/tutorials/Tutorial.svelte b/frontend/src/lib/components/tutorials/Tutorial.svelte index fc95141f5c..d4c99482a2 100644 --- a/frontend/src/lib/components/tutorials/Tutorial.svelte +++ b/frontend/src/lib/components/tutorials/Tutorial.svelte @@ -1,155 +1,101 @@ {#if tutorial} diff --git a/frontend/src/lib/components/tutorials/TutorialControls.svelte b/frontend/src/lib/components/tutorials/TutorialControls.svelte index 826e66d151..dd15cf4151 100644 --- a/frontend/src/lib/components/tutorials/TutorialControls.svelte +++ b/frontend/src/lib/components/tutorials/TutorialControls.svelte @@ -1,51 +1,39 @@
    {#if activeIndex === 0} -
  • UI is not interactive during tutorial, press next at every step
    • -
  • You can use the arrow keys to navigate
    • +
  • UI is not interactive during the tour, press next at every step
    • +
  • You can use the arrow keys to navigate
    •  {/if}
    - {#if activeIndex !== undefined && totalSteps !== undefined} -
    - Step {activeIndex + 1} of {totalSteps} -
    - {/if} +
    + Step {activeIndex + 1} of {totalSteps} +
    -
    diff --git a/frontend/src/lib/components/tutorials/TutorialProgressBar.svelte b/frontend/src/lib/components/tutorials/TutorialProgressBar.svelte deleted file mode 100644 index 5084299a7e..0000000000 --- a/frontend/src/lib/components/tutorials/TutorialProgressBar.svelte +++ /dev/null @@ -1,29 +0,0 @@ - - -
    -
    -
    - Progress: {completed} of {total} {label} completed -
    -
    {progressPercentage}%
    -
    -
    -
    -
    -
    - diff --git a/frontend/src/lib/components/tutorials/TutorialRouter.svelte b/frontend/src/lib/components/tutorials/TutorialRouter.svelte deleted file mode 100644 index 80c8938eff..0000000000 --- a/frontend/src/lib/components/tutorials/TutorialRouter.svelte +++ /dev/null @@ -1,64 +0,0 @@ - - -{#each tutorials as tutorial} - -{/each} - diff --git a/frontend/src/lib/components/tutorials/TutorialWrapper.svelte b/frontend/src/lib/components/tutorials/TutorialWrapper.svelte deleted file mode 100644 index 32b6fa212f..0000000000 --- a/frontend/src/lib/components/tutorials/TutorialWrapper.svelte +++ /dev/null @@ -1,36 +0,0 @@ - - -{#if Component} - {@const Comp = Component} - -{/if} - diff --git a/frontend/src/lib/components/tutorials/app/BackgroundRunnablesTutorial.svelte b/frontend/src/lib/components/tutorials/app/BackgroundRunnablesTutorial.svelte deleted file mode 100644 index 65fa99ed46..0000000000 --- a/frontend/src/lib/components/tutorials/app/BackgroundRunnablesTutorial.svelte +++ /dev/null @@ -1,91 +0,0 @@ - - - { - const steps: DriveStep[] = [ - { - element: '#app-editor-runnable-panel', - popover: { - title: 'Runnable panel', - description: - 'This is the runnable panel. Here you can add runnables to your app. Runnables are scripts that can be executed in the background. You can add as many runnables as you want.' - } - }, - { - element: '#create-background-runnable', - popover: { - title: 'Create a runnable', - description: - 'Click here to create a runnable. Runnables are scripts that can be executed in the background. You can add as many runnables as you want.', - onNextClick: () => { - clickButtonBySelector('#create-background-runnable') - setTimeout(() => driver.moveNext()) - } - } - }, - { - element: '#app-editor-empty-runnable', - popover: { - title: 'Empty runnable panel', - description: - 'This is the empty runnable panel. Here you can add runnables to your app. Runnables are scripts that can be executed in the background. You can add as many runnables as you want. You can also select a script or a flow from your workspace or the Hub.' - } - }, - - { - element: '#app-editor-backend-runnables', - popover: { - title: 'Backend runnables', - description: - 'Backend runnables are scripts that are executed on the server. They can be used to perform tasks that are not possible to be performed on the client. For example, you can use backend runnables to send emails, perform database operations, etc.' - } - }, - { - element: '#app-editor-frontend-runnables', - popover: { - title: 'Frontend runnables', - description: - 'Frontend scripts are executed in the browser and can manipulate the app context directly. You can also interact with components using component controls.', - onNextClick: () => { - setTimeout(() => { - driver.moveNext() - - updateProgress(index) - }) - } - } - } - ] - - // Remove steps if we want to skip them (excpet the first one) - - if (options?.skipStepsCount) { - steps.splice(1, options.skipStepsCount) - } - - return steps - }} -/> diff --git a/frontend/src/lib/components/tutorials/app/ConnectionTutorial.svelte b/frontend/src/lib/components/tutorials/app/ConnectionTutorial.svelte deleted file mode 100644 index 9ae02af92a..0000000000 --- a/frontend/src/lib/components/tutorials/app/ConnectionTutorial.svelte +++ /dev/null @@ -1,128 +0,0 @@ - - - [ - { - popover: { - title: 'Connection tutorial', - description: 'We will connect the input of a text component to an output.', - onNextClick: () => { - addComponent() - setTimeout(() => { - driver.moveNext() - }) - } - } - }, - { - element: `#component-input`, - popover: { - title: 'Data source', - description: - 'Here we can set the data source of the text component: it can be static, the result of an evaluation or the result of script or flow. We are going to connect the data source to an output.', - onNextClick: () => { - clickButtonBySelector('#component-input') - setTimeout(() => { - driver.moveNext() - }) - } - } - }, - { - element: '[data-connection-button] button[title="Connect"]', - popover: { - title: 'Connect the text component', - description: 'Click on the plug icon to connect the text component', - onNextClick: () => { - clickButtonBySelector('[data-connection-button] button[title="Connect"]') - setTimeout(() => { - driver.moveNext() - }) - } - } - }, - { - element: '#output-ctx', - popover: { - title: 'Select the output', - description: - "You can now select the output in the output menu. Let's select your email in the app context", - onNextClick: () => { - clickButtonBySelector('#output-ctx') - setTimeout(() => { - driver.moveNext() - }) - } - } - }, - { - element: '.val', - popover: { - title: 'Click on the output', - description: 'Simply click on the output to connect it', - onNextClick: () => { - clickButtonBySelector('.val') - setTimeout(() => { - driver.moveNext() - }) - } - } - }, - { - popover: { - title: 'Connection done', - description: 'You can now see the email output connected to the text component input', - onNextClick: () => { - updateProgress(index) - - setTimeout(() => { - driver.moveNext() - }) - } - } - } - ]} -/> diff --git a/frontend/src/lib/components/tutorials/app/ExpressionEvaluationTutorial.svelte b/frontend/src/lib/components/tutorials/app/ExpressionEvaluationTutorial.svelte deleted file mode 100644 index 1e96e8b121..0000000000 --- a/frontend/src/lib/components/tutorials/app/ExpressionEvaluationTutorial.svelte +++ /dev/null @@ -1,33 +0,0 @@ - - - [ - { - popover: { - title: 'Expression evaluation tutorial', - description: - 'Learn how to build our first branch to be executed on a condition. You can use arrow keys to navigate' - } - } - ]} -/> diff --git a/frontend/src/lib/components/tutorials/ignoredTutorials.ts b/frontend/src/lib/components/tutorials/ignoredTutorials.ts deleted file mode 100644 index 7a120b5e0c..0000000000 --- a/frontend/src/lib/components/tutorials/ignoredTutorials.ts +++ /dev/null @@ -1,3 +0,0 @@ -import { writable } from 'svelte/store' - -export const ignoredTutorials = writable([]) diff --git a/frontend/src/lib/components/tutorials/operatorTour.ts b/frontend/src/lib/components/tutorials/operatorTour.ts new file mode 100644 index 0000000000..5c4269e84d --- /dev/null +++ b/frontend/src/lib/components/tutorials/operatorTour.ts @@ -0,0 +1,47 @@ +import { UserService } from '$lib/gen' + +/** + * The tour's slot in the `tutorial_progress` bitmask. Slot 6 is reserved for it across + * versions: an operator who has already been through the tour must not meet it again, and + * a slot that another tutorial writes would read as finished on day one. + */ +const OPERATOR_TOUR_BIT = 6 + +/** URL parameter the sidebar entry uses to ask the home page for a run. */ +export const TOUR_PARAM = 'tour' +export const TOUR_PARAM_VALUE = 'operator' + +/** Long enough for the home page's tabs to exist before the first step points at one. */ +export const TOUR_START_DELAY_MS = 500 +/** Time for the sidebar to open before the last step points into it. */ +export const MENU_OPEN_DELAY_MS = 300 + +export async function hasSeenOperatorTour(): Promise { + // A failure answers "seen": the tour interrupts the page, and interrupting someone who + // has already been through it is worse than never offering it, which the sidebar entry + // covers anyway. + try { + const progress = (await UserService.getTutorialProgress()).progress ?? 0 + return (progress & (1 << OPERATOR_TOUR_BIT)) !== 0 + } catch (error) { + console.error('Could not read tutorial progress:', error) + return true + } +} + +export async function markOperatorTourSeen(): Promise { + try { + // Read-modify-write, because the row is shared: it carries every slot's state, and a + // write of this bit alone would clear the rest. `skipped_all` rides along for the same + // reason — and the handler rejects a body without it, whatever the generated type says. + const current = await UserService.getTutorialProgress() + await UserService.updateTutorialProgress({ + requestBody: { + progress: (current.progress ?? 0) | (1 << OPERATOR_TOUR_BIT), + skipped_all: current.skipped_all ?? false + } + }) + } catch (error) { + console.error('Could not record tutorial progress:', error) + } +} diff --git a/frontend/src/lib/components/tutorials/utils.ts b/frontend/src/lib/components/tutorials/utils.ts deleted file mode 100644 index 083e9712dd..0000000000 --- a/frontend/src/lib/components/tutorials/utils.ts +++ /dev/null @@ -1,328 +0,0 @@ -import type { FlowModule, OpenFlow } from '$lib/gen' -import { deepEqual } from 'fast-equals' -import { emptyApp } from '../apps/editor/appUtils' -import type { App } from '../apps/types' -import { findGridItem } from '../apps/editor/appUtilsCore' -import { isRunnableByName } from '../apps/inputType' -import { wait } from '$lib/utils' - -// Tutorial animation delay constants -export const DELAY_SHORT = 100 -export const DELAY_MEDIUM = 300 -export const DELAY_LONG = 500 -export const DELAY_ANIMATION = 1500 -export const DELAY_ANIMATION_LONG = 2500 -export const DELAY_TYPING = 50 -export const DELAY_CODE_CHAR = 2 -export const DELAY_CODE_NEWLINE = 5 - -export function setInputBySelector(selector: string, value: string) { - const input = document.querySelector(selector) as HTMLInputElement - - if (input) { - input.value = value - input.dispatchEvent(new Event('input', { bubbles: true })) - } -} - -export function clickButtonBySelector(selector: string) { - const button = document.querySelector(selector) as HTMLButtonElement - - if (button) { - button.click() - } -} - -export function clickFirstButtonBySelector(selector: string) { - const buttons = document.querySelector(selector) - const button = buttons?.childNodes[0] as HTMLButtonElement - - if (button) { - button.click() - } -} - -export function triggerPointerDown(selector: string) { - const elem = document.querySelector(selector) as HTMLElement - - if (elem) { - elem.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) - } -} - -export function selectOptionsBySelector(selector: string, value: string) { - const select = document.querySelector(selector) as HTMLSelectElement - - if (select) { - select.value = value - select.dispatchEvent(new Event('change', { bubbles: true })) - } -} - -export function isFlowTainted(flow: OpenFlow) { - return ( - flow.value.modules.length > 0 || Object.keys((flow?.schema?.properties as any) ?? {}).length > 0 - ) -} - -export function isAppTainted(app: App) { - if (app.hideLegacyTopBar === true) { - // An empty app should have only have a topbar and no hidden inline scripts - - if (Array.isArray(app.hiddenInlineScripts) && app.hiddenInlineScripts?.length > 0) { - return true - } - - // New apps have only a single component which is the topbar - if (Array.isArray(app.grid) && app.grid.length > 1) { - return true - } - - // Check if the current app is different from an empty app - return !deepEqual(app, emptyApp()) - } else { - // For older apps, - return !(app.grid?.length === 0 && app.hiddenInlineScripts?.length === 0) - } -} - -export function updateFlowModuleById( - flow: OpenFlow, - id: string, - callback: (module: FlowModule) => void -) { - const dfs = (modules: FlowModule[]) => { - for (const module of modules) { - if (module.id === id) { - callback(module) - return - } - - if (module.value.type === 'forloopflow') { - dfs(module.value.modules) - } else if (module.value.type === 'branchone') { - module.value.branches.forEach((branch) => dfs(branch.modules)) - } else if (module.value.type === 'branchall') { - module.value.branches.forEach((branch) => dfs(branch.modules)) - } - // AI agent tools are leaf nodes - no traversal needed - } - } - - dfs(flow.value.modules) -} - -export function updateBackgroundRunnableCode(app: App, index: number, newCode: string) { - const script = app.hiddenInlineScripts[index] - if (isRunnableByName(script) && script.inlineScript) { - script.inlineScript.content = newCode - } -} - -export function updateInlineRunnableCode(app: App, componentId: string, newCode: string) { - const gridItem = findGridItem(app, componentId) - if (gridItem?.data.componentInput?.type === 'runnable') { - if ( - isRunnableByName(gridItem.data.componentInput.runnable) && - gridItem.data.componentInput.runnable.inlineScript - ) { - gridItem.data.componentInput.runnable.inlineScript.content = newCode - } - } -} - -export function connectComponentSourceToOutput(app: App, componentId: string, targetId: string) { - const gridItem = findGridItem(app, componentId) - - if (gridItem) { - gridItem.data.componentInput = { - type: 'evalv2', - fieldType: 'object', - - expr: `${targetId}.result`, - connections: [ - { - componentId: targetId, - id: 'result' - } - ] - } - } -} - -export function connectInlineRunnableInputToComponentOutput( - app: App, - sourceComponentId: string, - sourceField: string, - targetComponentId: string, - targetField: string, - fieldType: string = 'text' -) { - const gridItem = findGridItem(app, sourceComponentId) - - if (gridItem?.data.componentInput?.type === 'runnable') { - // @ts-ignore - gridItem.data.componentInput.fields = { - [sourceField]: { - type: 'evalv2', - expr: `${targetComponentId}.${targetField}`, - fieldType: fieldType, - connections: [ - { - componentId: targetComponentId, - id: targetField - } - ] - } - } - } -} - -function elementExists(selector: string): boolean { - return !!document.querySelector(selector) -} - -export function waitForElementLoading( - selector: string, - callback: () => void, - interval: number = 50, - maxAttempts: number = 30 -): void { - let attempts = 0 - - const checkExistence = setInterval(() => { - if (elementExists(selector)) { - clearInterval(checkExistence) - callback() - } else if (attempts >= maxAttempts) { - clearInterval(checkExistence) - console.error('Element not found after multiple attempts.') - } - attempts++ - }, interval) -} - -// Helper function to move cursor to element (for continuous cursor movement in tutorials) -export async function moveCursorToElement( - cursor: HTMLElement, - element: HTMLElement, - duration: number = DELAY_ANIMATION -): Promise { - const rect = element.getBoundingClientRect() - cursor.style.transition = `all ${duration / 1000}s ease-in-out` - cursor.style.left = `${rect.left + rect.width / 2}px` - cursor.style.top = `${rect.top + rect.height / 2}px` - await wait(duration) -} - -// Helper function to create a fake cursor element for tutorial animations -export function createFakeCursor(): HTMLElement { - const fakeCursor = document.createElement('div') - fakeCursor.style.cssText = ` - position: fixed; - width: 20px; - height: 20px; - border-radius: 50%; - background-color: rgba(59, 130, 246, 0.8); - border: 2px solid white; - pointer-events: none; - z-index: 10000; - transition: all 1.5s ease-in-out; - ` - document.body.appendChild(fakeCursor) - return fakeCursor -} - -// Constants for cursor animation -const CURSOR_START_OFFSET = -100 -const CURSOR_CLICK_SCALE = 0.8 - -// Helper function to create and animate a fake cursor with start position -export async function createFakeCursorWithStart( - startElement: HTMLElement | null, - endElement: HTMLElement, - transitionDuration: number = 1.5 -): Promise { - const fakeCursor = createFakeCursor() - - const endRect = endElement.getBoundingClientRect() - let startX: number, startY: number - - if (startElement) { - const startRect = startElement.getBoundingClientRect() - startX = startRect.left + startRect.width / 2 - startY = startRect.top + startRect.height / 2 - } else { - startX = endRect.left + CURSOR_START_OFFSET - startY = endRect.top + endRect.height / 2 - } - - fakeCursor.style.left = `${startX}px` - fakeCursor.style.top = `${startY}px` - - await wait(DELAY_SHORT) - - fakeCursor.style.left = `${endRect.left + endRect.width / 2}px` - fakeCursor.style.top = `${endRect.top + endRect.height / 2}px` - - await wait(transitionDuration * 1000) - - return fakeCursor -} - -// Helper function to animate a fake cursor click -export async function animateFakeCursorClick( - element: HTMLElement, - transitionDuration: number = 1.5, - options?: { usePointerEvents?: boolean; startElement?: HTMLElement | null } -): Promise { - const fakeCursor = await createFakeCursorWithStart( - options?.startElement ?? null, - element, - transitionDuration - ) - await wait(DELAY_MEDIUM) - - // Animate click (shrink cursor briefly) - fakeCursor.style.transform = `scale(${CURSOR_CLICK_SCALE})` - await wait(DELAY_SHORT) - fakeCursor.style.transform = 'scale(1)' - await wait(DELAY_SHORT) - - // Trigger pointer events if needed (flow graph uses pointer events instead of click) - if (options?.usePointerEvents) { - element.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) - element.dispatchEvent(new PointerEvent('pointerup', { bubbles: true })) - } - - // Click the element - element.click() - await wait(DELAY_SHORT) - - // Remove fake cursor - fakeCursor.remove() -} - -// Helper function to animate cursor to element and click (for reusing a cursor across multiple clicks) -export async function animateCursorToElementAndClick( - cursor: HTMLElement, - element: HTMLElement, - startOffset: number = CURSOR_START_OFFSET -): Promise { - const rect = element.getBoundingClientRect() - - // Set initial position (off-screen to the left) - cursor.style.left = `${rect.left + startOffset}px` - cursor.style.top = `${rect.top + rect.height / 2}px` - await wait(DELAY_SHORT) - - // Animate to target position - cursor.style.left = `${rect.left + rect.width / 2}px` - cursor.style.top = `${rect.top + rect.height / 2}px` - await wait(DELAY_ANIMATION) - await wait(DELAY_MEDIUM) - - // Click on the element - element.click() - await wait(DELAY_SHORT) -} diff --git a/frontend/src/lib/components/tutorials/workspace/WorkspaceOnboardingOperatorTutorial.svelte b/frontend/src/lib/components/tutorials/workspace/WorkspaceOnboardingOperatorTutorial.svelte deleted file mode 100644 index 4251a67170..0000000000 --- a/frontend/src/lib/components/tutorials/workspace/WorkspaceOnboardingOperatorTutorial.svelte +++ /dev/null @@ -1,141 +0,0 @@ - - - { - const steps: DriveStep[] = [ - { - popover: { - title: 'Welcome to Windmill! 🎉', - description: - "Let's take a quick tour! We'll show you the three main tools you can use: Scripts, Flows, and Apps.", - onNextClick: () => { - // Wait a bit to ensure the page is fully rendered before moving to next step - setTimeout(() => { - // Try to find the script tab button - const scriptsButton = document.querySelector('[data-value="script"]') as HTMLElement | null - - if (scriptsButton) { - driver.moveNext() - } else { - // If we can't find the button, just move to next step anyway - driver.moveNext() - } - }, 100) - } - } - }, - { - popover: { - title: 'Scripts - Run automated tasks', - description: - 'Script Example

    Scripts are ready-to-use tasks that do things automatically for you.

    You can run scripts whenever you need them - like generating a report, sending notifications, or processing data.

    ', - onNextClick: async () => { - // Move to the next step (Flows) - setTimeout(() => { - const flowsButton = document.querySelector('[data-value="flow"]') as HTMLElement | null - - if (flowsButton) { - driver.moveNext() - } else { - driver.moveNext() - } - }, 100) - } - }, - element: '[data-value="script"]' - }, - { - popover: { - title: 'Flows - Run step-by-step processes', - description: - 'Flow

    Flows are processes that run multiple tasks in order, one after another.

    You can start a flow and watch it complete each step automatically - perfect for tasks that have multiple stages.

    ', - onNextClick: async () => { - // Move to the next step (Apps) - setTimeout(() => { - const appsButton = document.querySelector('[data-value="app"]') as HTMLElement | null - - if (appsButton) { - driver.moveNext() - } else { - driver.moveNext() - } - }, 100) - } - }, - element: '[data-value="flow"]' - }, - { - popover: { - title: 'Apps - Use custom tools', - description: - 'App

    Apps are easy-to-use tools with buttons, forms, and displays built just for your team.

    You can open an app to work with your data, fill out forms, or trigger tasks - no technical knowledge needed!

    ', - onNextClick: async () => { - // Move to the next step (cursor animation) - driver.moveNext() - } - }, - element: '[data-value="app"]' - }, - { - popover: { - title: 'Finally, the Menu section', - description: 'Explore available tabs where you can access your history of runs, your scheduled scripts, your tutorials progress etc.

    💡 Want to learn more? Access more tutorials from the Tutorials page in the main menu.

    ', - onNextClick: async () => { - // Find the target button and click it - const targetButton = document.querySelector('[role="menuitem"]') as HTMLElement | null - if (targetButton) { - targetButton.click() - } - - // Wait for menu to open - await wait(DELAY_MEDIUM) - - // Mark tutorial as complete - updateProgress(index) - driver.destroy() - - // Clean up URL parameter if present - if (page.url.searchParams.has('tutorial')) { - goto(`${base}/`, { replaceState: true }) - } - } - }, - element: '[role="menuitem"]' - } - ] - - return steps - }} -/> diff --git a/frontend/src/lib/components/tutorials/workspace/WorkspaceOnboardingTutorial.svelte b/frontend/src/lib/components/tutorials/workspace/WorkspaceOnboardingTutorial.svelte deleted file mode 100644 index 30307463a1..0000000000 --- a/frontend/src/lib/components/tutorials/workspace/WorkspaceOnboardingTutorial.svelte +++ /dev/null @@ -1,95 +0,0 @@ - - - { - const steps: DriveStep[] = [ - { - popover: { - title: 'Welcome to your Windmill workspace! 🎉', - description: - "Let's take a quick tour! We will show you the main sections of your workspace.", - onNextClick: async () => { - // The New menu button mounts once an async permission check resolves, so - // wait for it before highlighting it in the next step. - for (let i = 0; i < 20 && !document.querySelector('#create-new-button'); i++) { - await new Promise((resolve) => setTimeout(resolve, 100)) - } - driver.moveNext() - } - } - }, - { - popover: { - title: 'Create your first script', - description: - 'Programming Languages

    Open the New menu to create a script. Scripts turn code into tools. Write in Python, TypeScript, Go, Bash, SQL and more. Run them manually, on schedule, or via webhooks.

    ', - onNextClick: () => { - driver.moveNext() - } - }, - element: '#create-new-button' - }, - { - popover: { - title: 'Create your first flow', - description: - 'Flow

    The same New menu lets you create a flow. Flows orchestrate multiple scripts. Chain them together with branching, loops, and error handling to build complex workflows.

    ', - onNextClick: () => { - driver.moveNext() - } - }, - element: '#create-new-button' - }, - { - popover: { - title: 'Create your first app', - description: - 'App

    And from the New menu you can also create an app. Apps are custom UIs built with drag-and-drop. Combine tables, forms, charts, and buttons that trigger your scripts and flows. That\'s it for the tour!

    💡 Want to learn more? Access more tutorials from the Tutorials page in the main menu or in the Help submenu.

    ', - onNextClick: async () => { - // Mark tutorial as complete - updateProgress(index) - driver.destroy() - - // Clean up URL parameter if present - if (page.url.searchParams.has('tutorial')) { - goto(`${base}/`, { replaceState: true }) - } - } - }, - element: '#create-new-button' - } - ] - - return steps - }} -/> diff --git a/frontend/src/lib/components/workspaceSettings/SimpleCreateWorkspace.svelte b/frontend/src/lib/components/workspaceSettings/SimpleCreateWorkspace.svelte new file mode 100644 index 0000000000..67059c2fb8 --- /dev/null +++ b/frontend/src/lib/components/workspaceSettings/SimpleCreateWorkspace.svelte @@ -0,0 +1,237 @@ + + +{#if creating} +
    + + Creating {name.trim()}… +
    +{:else if advanced} + + + {#if leading} +
    {@render leading()}
    + {/if} +{:else} +
    + Workspace name + (nameEdited = true), + onkeydown: (e) => e.key === 'Enter' && create() + }} + /> + {#if problem && name.trim()} + {problem} + {/if} + {#if policyFailed} + + This instance's settings could not be read, so a workspace cannot be created yet. + + + {/if} + +
    +
    + {@render leading?.()} + + + +
    + +
    +
    +{/if} diff --git a/frontend/src/lib/hubProject.test.ts b/frontend/src/lib/hubProject.test.ts new file mode 100644 index 0000000000..ca1891110d --- /dev/null +++ b/frontend/src/lib/hubProject.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('./gen', () => ({ HubPublishService: {}, SettingService: {} })) +vi.mock('./components/icons', () => ({ appIconComponent: () => undefined })) + +import { hubProjectDescription } from './hubProject' + +describe('hubProjectDescription', () => { + it('prefers the description field when the hub has one', () => { + expect(hubProjectDescription({ description: ' Runs payroll. ', readme: '# Other' })).toBe( + 'Runs payroll.' + ) + }) + + it('reads the readme intro when it does not, which is every published project', () => { + expect( + hubProjectDescription({ + description: '', + readme: 'Audiences and campaigns,\nwith a sending engine.\n\n## Concepts\n\n- A flow' + }) + ).toBe('Audiences and campaigns, with a sending engine.') + }) + + it('skips a leading heading rather than stopping at it', () => { + expect( + hubProjectDescription({ + readme: '## Description\n\nManages Odoo records.\n\n## Usage\n\n1. Generate a key' + }) + ).toBe('Manages Odoo records.') + }) + + it('strips inline markdown', () => { + expect( + hubProjectDescription({ readme: 'A **bold** clone of [Bitly](https://bitly.com) with `js`.' }) + ).toBe('A bold clone of Bitly with js.') + }) + + it('cuts on a word boundary, so a long one reads as shortened not corrupted', () => { + const long = hubProjectDescription({ readme: 'lorem ipsum '.repeat(40) }) + expect(long.length).toBeLessThanOrEqual(321) + expect(long.endsWith('…')).toBe(true) + expect(long).not.toMatch(/lore…$/) + }) + + it('falls back to the summary when there is no prose at all', () => { + expect(hubProjectDescription({ readme: '## Usage\n', summary: 'Short links' })).toBe( + 'Short links' + ) + expect(hubProjectDescription({})).toBe('') + }) +}) diff --git a/frontend/src/lib/hubProject.ts b/frontend/src/lib/hubProject.ts index b58b338f7d..be108b2136 100644 --- a/frontend/src/lib/hubProject.ts +++ b/frontend/src/lib/hubProject.ts @@ -1,6 +1,6 @@ import type { Component } from 'svelte' import { appIconComponent } from '$lib/components/icons' -import { SettingService } from '$lib/gen' +import { HubPublishService, SettingService } from '$lib/gen' import { DEFAULT_HUB_BASE_URL } from '$lib/hub' import type { ImportProjectSummary } from '$lib/components/ImportProjectCard.svelte' @@ -85,3 +85,123 @@ const HUB_APP_ICON_ALIAS: Record = { postgres: 'postgresql' } export function hubAppIcon(app: string): Component | undefined { return appIconComponent(HUB_APP_ICON_ALIAS[app] ?? app) } + +/** One row of the hub's catalogue (`GET /projects`), which carries no item counts. */ +interface HubProjectListRow { + slug: string + name: string + summary: string + description: string + readme: string + author: string + apps: string[] + hasLogo: boolean + stars: number +} + +const DESCRIPTION_MAX = 320 + +/** + * What a project says about itself, in prose. + * + * The hub's `description` field is empty on every published project — the writing all + * goes in the readme — so the readme's opening paragraphs stand in. Everything from the + * first heading onwards is dropped: that is the "Windmill concepts demonstrated" / + * "Usage" material, which is documentation rather than a description. A readme that + * *starts* with a heading (`## Description`) has it skipped rather than treated as the + * end of the intro. + */ +export function hubProjectDescription(row: { + description?: string + readme?: string + summary?: string +}): string { + if (row.description?.trim()) return row.description.trim() + + const lines = (row.readme ?? '').split('\n') + let i = 0 + while (i < lines.length && (lines[i].trim() === '' || lines[i].startsWith('#'))) i++ + const intro: string[] = [] + for (; i < lines.length; i++) { + if (lines[i].startsWith('#')) break + intro.push(lines[i]) + } + + const text = intro + .join(' ') + // Inline markdown only — the block syntax is already gone with the headings. + .replace(/\[([^\]]+)\]\([^)]*\)/g, '$1') + .replace(/[*_`]/g, '') + .replace(/\s+/g, ' ') + .trim() + if (!text) return row.summary?.trim() ?? '' + if (text.length <= DESCRIPTION_MAX) return text + // Cut on a word boundary: a description sliced mid-word reads as corrupted rather + // than shortened. + const cut = text.slice(0, DESCRIPTION_MAX) + const lastSpace = cut.lastIndexOf(' ') + return `${(lastSpace > DESCRIPTION_MAX * 0.6 ? cut.slice(0, lastSpace) : cut).trimEnd()}…` +} + +/** + * A card in the template picker. Everything it shows comes from the catalogue listing, + * so a whole page of cards costs one request; the item counts, which only the import + * step needs, are fetched per project by `fetchHubProject` when one is picked. + * `id` is what `InfiniteList` dedupes rows by. + */ +export interface HubProjectPick { + id: string + slug: string + name: string + summary: string + description: string + author: string + apps: string[] + logoUrl?: string + iconApps: string[] + stars: number +} + +let catalogue: { workspace: string; projects: Promise } | undefined + +/** + * Every published project, most-starred first, fetched once per workspace and held for + * the life of the page. + * + * Through the workspace-scoped proxy rather than straight at the hub the way + * `fetchHubProject` goes: the catalogue endpoint sends no `Access-Control-Allow-Origin`, + * so the browser cannot read it directly. + */ +export function hubProjectCatalogue(workspace: string): Promise { + if (catalogue?.workspace !== workspace) { + const projects = loadCatalogue(workspace).catch((e) => { + // A cached rejection would make the failure permanent for the whole session; + // dropping it lets the next open try again. + if (catalogue?.projects === projects) catalogue = undefined + throw e + }) + catalogue = { workspace, projects } + } + return catalogue.projects +} + +async function loadCatalogue(workspace: string): Promise { + const raw = await HubPublishService.listHubProjects({ workspace }) + const rows = ((typeof raw === 'string' ? JSON.parse(raw) : raw)?.projects ?? + []) as HubProjectListRow[] + const hub = await hubBrowserUrl() + return rows + .map((row) => ({ + id: row.slug, + slug: row.slug, + name: row.name, + summary: row.summary, + description: hubProjectDescription(row), + author: row.author, + apps: row.apps ?? [], + logoUrl: row.hasLogo ? `${hub}/projects/${encodeURIComponent(row.slug)}/logo` : undefined, + iconApps: row.apps ?? [], + stars: row.stars ?? 0 + })) + .sort((a, b) => b.stars - a.stars || a.name.localeCompare(b.name)) +} diff --git a/frontend/src/lib/importWizard/abandon.test.ts b/frontend/src/lib/importWizard/abandon.test.ts index abf0ff61b9..d240c8147d 100644 --- a/frontend/src/lib/importWizard/abandon.test.ts +++ b/frontend/src/lib/importWizard/abandon.test.ts @@ -150,6 +150,30 @@ describe('abandoning mid-import', () => { expect(run.itemResults.length).toBe(3) }) + // What a caller acting on the run's leftovers depends on: `abandon()` only stops the next + // phase, so a reload issued when it is called reads the workspace while the request already + // sent is still landing. `whenIdle()` is the difference between reloading then and after. + it('whenIdle resolves only once the abandoned run has stopped writing', async () => { + const run = new ImportExecution(PLAN, deps) + let idleResolved = false + hooks.afterFirstItem = () => { + run.abandon() + void run.whenIdle().then(() => (idleResolved = true)) + // Still inside the run: the promise must not have resolved yet. + expect(run.running).toBe(true) + expect(idleResolved).toBe(false) + } + await run.run() + await run.whenIdle() + expect(run.running).toBe(false) + expect(idleResolved).toBe(true) + }) + + it('whenIdle resolves immediately when no run is in flight', async () => { + const run = new ImportExecution(PLAN, deps) + await expect(run.whenIdle()).resolves.toBeUndefined() + }) + it('stops the migrate row spinning when it is abandoned mid-migration', async () => { const run = new ImportExecution(PLAN, depsWithMigration) // After `onMigrationsStart`, which is where the row is actually set to running — diff --git a/frontend/src/lib/importWizard/execution.svelte.ts b/frontend/src/lib/importWizard/execution.svelte.ts index 6a2c39b0b6..12952525b5 100644 --- a/frontend/src/lib/importWizard/execution.svelte.ts +++ b/frontend/src/lib/importWizard/execution.svelte.ts @@ -229,6 +229,29 @@ export class ImportExecution { */ async run(): Promise { if (this.running) return + const settled = this.#runInternal() + // Handled here so an abandoned or failed run does not surface as an unhandled + // rejection through `whenIdle()`, but still reported: `#runInternal` has no `catch` of + // its own, and a throw outside its inner ones leaves a stalled run with nothing on + // screen — the console is the only place that says why. + this.#idle = settled.catch((error) => console.error('import run failed:', error)) + return settled + } + + /** + * Resolves when the run in flight at the moment of the call is no longer writing — + * immediately when there is none. Callers that act on what a run left behind need this + * rather than a poll on `running`: `abandon()` stops the run at the next phase boundary, + * so the request already sent lands after it, and reading the workspace before then reads + * it mid-write. A caller that holds the promise across the start of a *second* run is + * resolved by the first, so re-read it if the surface stays open. + */ + whenIdle(): Promise { + return this.#idle + } + #idle: Promise = Promise.resolve() + + async #runInternal(): Promise { this.#abandoned = false this.running = true runState.active = true diff --git a/frontend/src/lib/importWizard/setupStep.svelte.ts b/frontend/src/lib/importWizard/setupStep.svelte.ts new file mode 100644 index 0000000000..17f63cbd4f --- /dev/null +++ b/frontend/src/lib/importWizard/setupStep.svelte.ts @@ -0,0 +1,73 @@ +import { WorkspaceService } from '$lib/gen' +import type { ImportExecution } from './execution.svelte' + +/** + * Whether a finished import leaves a setup step behind it, and whether that is still + * being decided. + * + * Known only once the run has fetched the export and the destination's data tables can + * be compared against it, so it is false for the whole wizard until the import + * finishes — which is exactly when it is first read. `undecided` matters as much as + * `needed`: without it the run reads as finished with no fourth step, and Finish leaves + * before the check comes back and discovers a data table that is missing. + * + * Shared by the wizard route and the in-workspace modal so the two cannot disagree + * about whether an import is over. + */ +export function useSetupStep( + getExecution: () => ImportExecution | undefined, + getWorkspace: () => string | undefined +) { + let needed = $state(false) + let undecided = $state(false) + + $effect(() => { + const execution = getExecution() + const names = execution?.datatableNames ?? [] + const workspace = getWorkspace() + if (!execution?.done || !workspace) { + needed = false + undecided = false + return + } + // `resourceCount` is the referenced subset — the resources something in the project + // points at — and each one arrives as an empty stub, so any project that has them has + // something to fill in. The step itself re-checks and shows only what is genuinely + // outstanding, which is what makes a re-import quiet. + if (execution.resourceCount > 0) { + needed = true + undecided = false + return + } + if (names.length === 0) { + needed = false + undecided = false + return + } + let cancelled = false + undecided = true + void WorkspaceService.listDataTables({ workspace }) + .then((tables) => { + if (cancelled) return + const present = new Set(tables.map((t) => t.name)) + needed = names.some((n) => !present.has(n)) + }) + .catch(() => { + // Can't tell — don't invent a step the user then cannot complete. + if (!cancelled) needed = false + }) + .finally(() => { + if (!cancelled) undecided = false + }) + return () => (cancelled = true) + }) + + return { + get needed() { + return needed + }, + get undecided() { + return undecided + } + } +} diff --git a/frontend/src/lib/refreshUser.ts b/frontend/src/lib/refreshUser.ts index de20fa5d56..4c6b47e8f3 100644 --- a/frontend/src/lib/refreshUser.ts +++ b/frontend/src/lib/refreshUser.ts @@ -1,23 +1,39 @@ import { get } from 'svelte/store' -import { CancelablePromise, UserService, type GlobalUserInfo } from '$lib/gen' +import { CancelablePromise, CancelError, UserService, type GlobalUserInfo } from '$lib/gen' import { superadmin, devopsRole } from './stores.js' let promise: CancelablePromise | null = null -async function _refreshSuperadmin(): Promise { - let shouldFetch = get(superadmin) == undefined || get(devopsRole) == undefined +/** + * `force` asks the server even when the stores already hold an answer. Worth it where a wrong + * answer changes what the page offers rather than how it looks: a logged-out load sets both + * stores to `false` — the request 401s — and without `force` nothing asks again for the rest + * of the session, so the user who signs in next reads as neither superadmin nor devops. + */ +async function _refreshSuperadmin(opts?: { force?: boolean }): Promise { + let shouldFetch = opts?.force || get(superadmin) == undefined || get(devopsRole) == undefined if (!shouldFetch) return undefined promise?.cancel() - promise = UserService.globalWhoami() + // Held locally so the check at the end can tell this request from a later caller's, which + // by then owns `promise`. + const mine = UserService.globalWhoami() + promise = mine try { - const me = await promise + const me = await mine superadmin.set(me.super_admin ? me.email : false) devopsRole.set(me.devops || me.super_admin ? me.email : false) } catch (error) { - superadmin.set(false) - devopsRole.set(false) - console.error('error refreshing superadmin/devops role', error) + // A cancellation says nothing about this user, so it must not be written down as an + // answer: `clearStores` cancels on logout, and a second caller cancels the first — and + // `false` here is precisely the stale state `force` exists to get out of. + if (!(error instanceof CancelError)) { + superadmin.set(false) + devopsRole.set(false) + console.error('error refreshing superadmin/devops role', error) + } } - promise = null + // Only if nobody has started another: clearing a live request's handle would put it beyond + // the reach of `cancel()`, and it would then land on a session that had been cleared. + if (promise === mine) promise = null } export const refreshSuperadmin = Object.assign(_refreshSuperadmin, { diff --git a/frontend/src/lib/stores.ts b/frontend/src/lib/stores.ts index ed5bc6f025..229ecff7ad 100644 --- a/frontend/src/lib/stores.ts +++ b/frontend/src/lib/stores.ts @@ -68,8 +68,6 @@ export function clearWorkspaceFromStorage() { sessionStorage.removeItem('workspace') } -export const tutorialsToDo = writable([]) -export const skippedAll = writable(false) export const globalEmailInvite = writable('') export const awarenessStore = writable>(undefined) export const enterpriseLicense = writable(undefined) @@ -120,6 +118,10 @@ export const superadmin = writable(undefined) export const devopsRole = writable(undefined) export const lspTokenStore = writable(undefined) export const hubBaseUrlStore = writable(DEFAULT_HUB_BASE_URL) +// Whether the store above is the instance's answer or still the default it was seeded with. +// It reads as the public hub either way, which is fine for a link and wrong for anything +// deciding what may be reported about a hub — those must treat unknown as private. +export const hubBaseUrlKnown = writable(false) export const wsBaseUrlStore = writable(undefined) export const disableHubStore = writable(false) // What a superadmin standing in a workspace they are not a member of needs to see it as a @@ -333,8 +335,6 @@ export const workspaceColor: Readable = derived( } ) -export const isCurrentlyInTutorial: StateStore = createState({ val: false }) - export function getFlatTableNamesFromSchema(dbSchema: DBSchema | undefined): string[] { const schema = dbSchema?.schema ?? {} const tableNames: string[] = [] diff --git a/frontend/src/lib/tutorialUtils.ts b/frontend/src/lib/tutorialUtils.ts deleted file mode 100644 index 02222138e8..0000000000 --- a/frontend/src/lib/tutorialUtils.ts +++ /dev/null @@ -1,224 +0,0 @@ -import { get } from 'svelte/store' -import { tutorialsToDo, skippedAll } from './stores' -import { UserService } from './gen' -import { TUTORIALS_CONFIG } from './tutorials/config' - -/** - * LocalStorage key for tracking if the tutorial banner has been dismissed. - * Shared between tutorialUtils and TutorialBanner component. - */ -export const TUTORIAL_BANNER_DISMISSED_KEY = 'tutorial_banner_dismissed' - -/** - * Get the maximum tutorial index from the config. - * This ensures we don't hardcode the max ID and it automatically updates when tutorials are added. - */ -function getMaxTutorialId(): number { - let maxId = 0 - for (const tab of Object.values(TUTORIALS_CONFIG)) { - for (const tutorial of tab.tutorials) { - if (tutorial.index !== undefined && tutorial.index > maxId) { - maxId = tutorial.index - } - } - } - return maxId -} - -const MAX_TUTORIAL_ID = getMaxTutorialId() - -/** - * Helper function to calculate tutorial progress for a given set of tutorial indexes. - * Returns total count. For completed count, use in component with reactive store access. - */ -export function getTutorialProgressTotal(tutorialIndexes: Record): number { - return Object.values(tutorialIndexes).length -} - -/** - * Helper function to calculate completed tutorials count. - * Must be called with current tutorialsToDo array. - */ -export function getTutorialProgressCompleted( - tutorialIndexes: Record, - tutorialsToDoArray: number[] -): number { - return Object.values(tutorialIndexes).filter((index) => !tutorialsToDoArray.includes(index)) - .length -} - -export async function updateProgress(id: number) { - const bef = get(tutorialsToDo) - const aft = bef.filter((x) => x != id) - tutorialsToDo.set(aft) - skippedAll.set(false) // Mark as not skipped when completing a tutorial - let bits = 0 - for (let i = 0; i <= MAX_TUTORIAL_ID; i++) { - let mask = 1 << i - if (!aft.includes(i)) { - bits = bits | mask - } - } - await UserService.updateTutorialProgress({ requestBody: { progress: bits, skipped_all: false } }) -} - -export async function skipAllTodos() { - let bits = 0 - for (let i = 0; i <= MAX_TUTORIAL_ID; i++) { - let mask = 1 << i - bits = bits | mask - } - tutorialsToDo.set([]) - skippedAll.set(true) - - await UserService.updateTutorialProgress({ requestBody: { progress: bits, skipped_all: true } }) -} - -export async function resetAllTodos() { - let todos: number[] = [] - for (let i = 0; i <= MAX_TUTORIAL_ID; i++) { - todos.push(i) - } - tutorialsToDo.set(todos) - skippedAll.set(false) - - await UserService.updateTutorialProgress({ requestBody: { progress: 0, skipped_all: false } }) -} - -/** - * Skip (mark as complete) all tutorials in a specific set of indexes - */ -export async function skipTutorialsByIndexes(tutorialIndexes: number[]) { - const currentTodos = get(tutorialsToDo) - const aft = currentTodos.filter((x) => !tutorialIndexes.includes(x)) - tutorialsToDo.set(aft) - - // Get current progress bits - const currentResponse = await UserService.getTutorialProgress() - let bits: number = currentResponse.progress ?? 0 - - // Set bits for the specified indexes - for (const index of tutorialIndexes) { - const mask = 1 << index - bits = bits | mask - } - - // Only set skipped_all to true if ALL tutorials are now complete - const allComplete = aft.length === 0 - await UserService.updateTutorialProgress({ - requestBody: { - progress: bits, - skipped_all: allComplete - } - }) -} - -/** - * Reset (mark as incomplete) all tutorials in a specific set of indexes - */ -export async function resetTutorialsByIndexes(tutorialIndexes: number[]) { - const currentTodos = get(tutorialsToDo) - const aft = [...new Set([...currentTodos, ...tutorialIndexes])] - tutorialsToDo.set(aft) - skippedAll.set(false) - - // Get current progress bits - const currentResponse = await UserService.getTutorialProgress() - let bits: number = currentResponse.progress ?? 0 - - // Clear bits for the specified indexes - for (const index of tutorialIndexes) { - const mask = 1 << index - bits = bits & ~mask - } - - await UserService.updateTutorialProgress({ - requestBody: { - progress: bits, - skipped_all: false - } - }) -} - -/** - * Update a single tutorial's completion status by index - */ -async function updateTutorialStatusByIndex(tutorialIndex: number, completed: boolean) { - const currentTodos = get(tutorialsToDo) - const isInTodos = currentTodos.includes(tutorialIndex) - - // Only update if the status needs to change - // isInTodos = true means NOT completed, isInTodos = false means completed - // So if completed === !isInTodos, we're already in the desired state - if (completed === !isInTodos) { - return // Already in the desired state - } - - // Update todos list - const aft = completed - ? currentTodos.filter((x) => x !== tutorialIndex) - : [...currentTodos, tutorialIndex] - tutorialsToDo.set(aft) - skippedAll.set(false) - - // Get current progress bits - const currentResponse = await UserService.getTutorialProgress() - let bits: number = currentResponse.progress ?? 0 - - // Update bit for this tutorial index - const mask = 1 << tutorialIndex - bits = completed ? bits | mask : bits & ~mask - - await UserService.updateTutorialProgress({ - requestBody: { - progress: bits, - skipped_all: false - } - }) -} - -/** - * Reset (mark as incomplete) a single tutorial by index - */ -export async function resetTutorialByIndex(tutorialIndex: number) { - await updateTutorialStatusByIndex(tutorialIndex, false) -} - -/** - * Mark a single tutorial as completed by index - */ -export async function completeTutorialByIndex(tutorialIndex: number) { - await updateTutorialStatusByIndex(tutorialIndex, true) -} - -export async function syncTutorialsTodos() { - const response = await UserService.getTutorialProgress() - const bits: number = response.progress! - const skipped: boolean = response.skipped_all ?? false - const todos: number[] = [] - for (let i = 0; i <= MAX_TUTORIAL_ID; i++) { - let mask = 1 << i - if ((bits & mask) == 0) { - todos.push(i) - } - } - tutorialsToDo.set(todos) - skippedAll.set(skipped) -} - -export function tutorialInProgress() { - const svg = document.getElementsByClassName('driver-overlay driver-overlay-animated') - - return svg.length > 0 -} - -/** - * Check if tutorials should be hidden from the main menu. - * Returns true if all tutorials are completed OR user skipped all. - */ -export function shouldHideTutorialsFromMainMenu(): boolean { - const todos = get(tutorialsToDo) - const skipped = get(skippedAll) - // Hide if all tutorials are completed OR user skipped all - return todos.length === 0 || skipped -} diff --git a/frontend/src/lib/tutorials/config.ts b/frontend/src/lib/tutorials/config.ts deleted file mode 100644 index 082dc0473d..0000000000 --- a/frontend/src/lib/tutorials/config.ts +++ /dev/null @@ -1,159 +0,0 @@ -import type { ComponentType } from 'svelte' -import { Workflow, GraduationCap, Wrench, PlayCircle, Link2, History } from 'lucide-svelte' -import { base } from '$lib/base' -import type { Role } from './roleUtils' - -export interface TutorialConfig { - id: string - icon: ComponentType - title: string - description: string - onClick: () => void - index?: number // Bitmask index in the database (for progress tracking) - active?: boolean // Whether this tutorial is active and should be displayed (default: true) - comingSoon?: boolean - roles?: Role[] // Roles that can access this tutorial (if not specified, available to everyone) - order?: number -} - -export interface TabConfig { - label: string - tutorials: TutorialConfig[] - roles?: Role[] // Roles that can access this tab category (if not specified, available to everyone) - progressBar?: boolean // Whether to display the progress bar for this tab (default: true) - active?: boolean // Whether this tab category is active and should be displayed (default: true) -} - -export type TabId = 'quickstart' | 'app_editor' - -/** - * Get tutorial index from config by tutorial ID. - * Throws an error if the tutorial or its index is not found. - */ -export function getTutorialIndex(id: string): number { - for (const tab of Object.values(TUTORIALS_CONFIG)) { - const tutorial = tab.tutorials.find((t) => t.id === id) - if (tutorial?.index !== undefined) return tutorial.index - } - throw new Error(`Tutorial index not found for id: ${id}. Make sure the tutorial has an index defined in config.`) -} - -// Available roles : developer, admin, operator - -export const TUTORIALS_CONFIG: Record = { - quickstart: { - label: 'Quickstart', - roles: ['admin', 'developer', 'operator'], - progressBar: true, - active: true, - tutorials: [ - { - id: 'workspace-onboarding', - icon: GraduationCap, - title: 'Workspace onboarding', - description: 'Discover the basics of Windmill with a quick tour of the workspace.', - onClick: () => { - window.location.href = `${base}/?tutorial=workspace-onboarding` - }, - index: 1, - active: true, - comingSoon: false, - roles: ['developer', 'admin'], - order: 1 - }, - { - id: 'flow-live-tutorial', - icon: Workflow, - title: 'Build a flow', - description: 'Learn how to build workflows in Windmill with our interactive tutorial.', - onClick: () => { - window.location.href = `${base}/flows/add?tutorial=flow-live-tutorial` - }, - index: 2, - active: true, - comingSoon: false, - roles: ['developer', 'admin'], - order: 2 - }, - { - id: 'troubleshoot-flow', - icon: Wrench, - title: 'Fix a broken flow', - description: 'Learn how to monitor and debug your script and flow executions.', - onClick: () => { - window.location.href = `${base}/flows/add?tutorial=troubleshoot-flow` - }, - index: 3, - active: true, - comingSoon: false, - roles: ['admin','developer'], - order: 3 - }, - { - id: 'runs-tutorial', - icon: History, - title: 'Discover your monitoring dashboard', - description: 'Learn how to monitor, filter, and manage your script and flow executions.', - onClick: () => { - window.location.href = `${base}/runs?tutorial=runs-tutorial` - }, - index: 7, - active: true, - comingSoon: false, - roles: ['admin', 'developer','operator'], - order: 4 - }, - { - id: 'workspace-onboarding-operator', - icon: GraduationCap, - title: 'Workspace onboarding', - description: 'Discover the basics of Windmill with a quick tour of the workspace.', - onClick: () => { - window.location.href = `${base}/?tutorial=workspace-onboarding-operator` - }, - index: 6, - active: true, - comingSoon: false, - roles: ['operator'], - order: 1 - }, - ] - }, - app_editor: { - label: 'App Editor', - roles: ['developer', 'admin'], - progressBar: false, - active: true, - tutorials: [ - { - id: 'backgroundrunnables', - icon: PlayCircle, - title: 'Background runnables', - description: 'Learn how to create and use background runnables in your apps.', - onClick: () => { - window.location.href = `${base}/apps/add?tutorial=backgroundrunnables` - }, - index: 4, - active: true, - comingSoon: false, - roles: ['developer','admin'], - order: 4 - }, - { - id: 'connection', - icon: Link2, - title: 'Connection', - description: 'Learn how to connect component inputs to outputs in your apps.', - onClick: () => { - window.location.href = `${base}/apps/add?tutorial=connection` - }, - index: 5, - active: true, - comingSoon: false, - roles: ['developer', 'admin'], - order: 5 - } - ] - } -} as const - diff --git a/frontend/src/lib/tutorials/roleUtils.ts b/frontend/src/lib/tutorials/roleUtils.ts deleted file mode 100644 index a727fca8d3..0000000000 --- a/frontend/src/lib/tutorials/roleUtils.ts +++ /dev/null @@ -1,68 +0,0 @@ -import type { UserExt } from '$lib/stores' - -export type Role = 'admin' | 'developer' | 'operator' - -/** - * Get the effective role of a user based on their database flags. - * - Admin: user.is_admin === true - * - Operator: user.operator === true (and not admin) - * - Developer: default (neither admin nor operator) - */ -export function getUserEffectiveRole(user: UserExt | null | undefined): Role | null { - if (!user) return null - if (user.is_admin) return 'admin' - if (user.operator) return 'operator' - return 'developer' -} - -/** - * Check if a role has access to a required role. - * This is the core role-checking logic used by both normal and preview modes. - */ -function checkRoleMatch( - userRole: Role, - requiredRole: Role -): boolean { - if (requiredRole === 'admin') return userRole === 'admin' - if (requiredRole === 'operator') return userRole === 'operator' || userRole === 'admin' - if (requiredRole === 'developer') return userRole === 'developer' || userRole === 'admin' - return false -} - -/** - * Check if a user or preview role has access based on a roles array. - * This is the unified function that handles both normal user access and admin preview mode. - */ -export function hasRoleAccess( - user: UserExt | null | undefined, - roles?: Role[], - previewRole?: Role -): boolean { - // No roles specified = available to everyone - if (!roles || roles.length === 0) return true - - // If previewRole is provided, use it (admin preview mode) - // Otherwise, derive role from user - const effectiveRole = previewRole ?? getUserEffectiveRole(user) - if (!effectiveRole) return false - - // Check if effective role has any of the required roles - return roles.some((role) => checkRoleMatch(effectiveRole, role)) -} - -/** - * Check if a preview role has access based on a roles array. - * Used by admins to preview what other roles can see. - * Uses exact role matching - only shows tutorials explicitly marked for the preview role. - */ -export function hasRoleAccessForPreview( - previewRole: Role, - roles?: Role[] -): boolean { - // No roles specified = available to everyone - if (!roles || roles.length === 0) return true - - // Exact role match - tutorial must explicitly include the preview role - return roles.includes(previewRole) -} - diff --git a/frontend/src/lib/utils/featureUsage.test.ts b/frontend/src/lib/utils/featureUsage.test.ts index ef60cd2b36..1857efb0c0 100644 --- a/frontend/src/lib/utils/featureUsage.test.ts +++ b/frontend/src/lib/utils/featureUsage.test.ts @@ -1,10 +1,34 @@ -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('$lib/gen', () => ({ OpenAPI: { BASE: '/api' } })) -vi.mock('$lib/stores', () => ({ workspaceStore: { subscribe: () => () => {} } })) + +// Stores `get()` can read, so a test can say which hub the instance points at and whether +// the instance has answered at all. +const hubBaseUrl = vi.hoisted(() => { + const readable = (initial: T) => { + let value = initial + return { + set: (v: T) => (value = v), + store: { + subscribe: (run: (v: T) => void) => { + run(value) + return () => {} + } + } + } + } + return { url: readable('https://hub.windmill.dev'), known: readable(true) } +}) + +vi.mock('$lib/stores', () => ({ + workspaceStore: { subscribe: () => () => {} }, + hubBaseUrlStore: hubBaseUrl.url.store, + hubBaseUrlKnown: hubBaseUrl.known.store +})) import { createFeatureUsageBuffer, + hubProjectUsageKey, hubScriptUsageKey, type FeatureUsageEventPayload } from './featureUsage' @@ -107,3 +131,50 @@ describe('hubScriptUsageKey', () => { ).toBe('acme/list_a_user_s_items_sorted') }) }) + +describe('hubProjectUsageKey', () => { + // The fixture is module-level and mutable, so each case states the world it needs rather + // than inheriting whatever the case above it left behind. + beforeEach(() => { + hubBaseUrl.url.set('https://hub.windmill.dev') + hubBaseUrl.known.set(true) + }) + + it('reports the slug for every spelling of the public hub', () => { + for (const hub of [ + 'https://hub.windmill.dev', + 'http://hub.windmill.dev/', + 'HTTPS://hub.windmill.dev', + 'https://HUB.WINDMILL.DEV', + 'https://hub.windmill.dev:443', + ' https://hub.windmill.dev ' + ]) { + hubBaseUrl.url.set(hub) + expect(hubProjectUsageKey('stripe-invoices'), hub).toBe('stripe-invoices') + } + }) + + it('answers private until the instance has said which hub it points at', () => { + // The store is seeded with the public hub, so a settings read that failed must not + // read as permission to report the name. + hubBaseUrl.known.set(false) + expect(hubProjectUsageKey('acme-payroll')).toBe('private') + hubBaseUrl.known.set(true) + expect(hubProjectUsageKey('acme-payroll')).toBe('acme-payroll') + }) + + it("keeps a private hub's project names off the wire", () => { + // The slug is the customer's own content on an instance running its own hub, and the + // disclosure only claims public project names. + for (const hub of [ + 'https://hub.internal.example', + 'https://hub.windmill.dev.evil.example', + 'https://windmill.dev', + 'hub.windmill.dev', + 'not a url' + ]) { + hubBaseUrl.url.set(hub) + expect(hubProjectUsageKey('acme-payroll'), hub).toBe('private') + } + }) +}) diff --git a/frontend/src/lib/utils/featureUsage.ts b/frontend/src/lib/utils/featureUsage.ts index 793d137593..31b33153cf 100644 --- a/frontend/src/lib/utils/featureUsage.ts +++ b/frontend/src/lib/utils/featureUsage.ts @@ -1,7 +1,7 @@ import { get } from 'svelte/store' import { OpenAPI } from '$lib/gen' -import { workspaceStore } from '$lib/stores' -import { PRIVATE_HUB_MIN_VERSION } from '$lib/hub' +import { hubBaseUrlKnown, hubBaseUrlStore, workspaceStore } from '$lib/stores' +import { DEFAULT_HUB_BASE_URL, PRIVATE_HUB_MIN_VERSION } from '$lib/hub' // Anonymous product-usage counters (e.g. AI session activity), batched into the // backend `feature_usage` accumulator. Only aggregated counts ever leave the @@ -187,3 +187,34 @@ export function hubScriptUsageKey(script: { if (!app) return PRIVATE_HUB_KEY return (summary ? `${app}/${summary}` : app).slice(0, 100) } + +/** + * A hub project's slug is only reportable when it names something on the public hub. An + * instance pointed at its own hub imports its own projects, whose names are the customer's + * content — the same reason `hubScriptUsageKey` collapses a private script to `private`, + * and what the disclosure means by "the name of any public hub project". + * + * Compared by host, so the port, scheme and trailing slash an operator may have typed do + * not decide it. Anything unparseable, and anything not yet read, answers private. + */ +export function hubProjectUsageKey(slug: string): string { + // `hubBaseUrlKnown` and not the URL alone: the store is seeded with the public hub, so an + // instance whose setting could not be read would otherwise report its own project names. + if (!get(hubBaseUrlKnown) || !isPublicHub(get(hubBaseUrlStore))) return PRIVATE_HUB_KEY + return slug.slice(0, 100) +} + +function isPublicHub(hub: string): boolean { + const host = (url: string): string | undefined => { + try { + const parsed = new URL(url.trim()) + return parsed.protocol === 'http:' || parsed.protocol === 'https:' + ? parsed.hostname.replace(/\.$/, '').toLowerCase() + : undefined + } catch { + return undefined + } + } + const configured = host(hub) + return configured !== undefined && configured === host(DEFAULT_HUB_BASE_URL) +} diff --git a/frontend/src/lib/workspaceCreation.test.ts b/frontend/src/lib/workspaceCreation.test.ts new file mode 100644 index 0000000000..975d1599b5 --- /dev/null +++ b/frontend/src/lib/workspaceCreation.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it, vi } from 'vitest' + +// The module reaches the API for the username policy and the workspace list; the name +// helper touches neither. `getGlobal` is a spy so the policy's failure path can be driven. +const getGlobal = vi.fn() +vi.mock('./gen', () => ({ + SettingService: { + get getGlobal() { + return getGlobal + } + }, + UserService: {}, + WorkspaceService: {} +})) +vi.mock('./stores', () => ({ usersWorkspaceStore: { set: () => {} } })) +vi.mock('./storeUtils', () => ({ switchWorkspace: () => {} })) +vi.mock('./cloud', () => ({ isCloudHosted: () => false })) + +import { defaultWorkspaceName, loadUsernamePolicy, usernameFromName } from './workspaceCreation' + +describe('defaultWorkspaceName', () => { + it('names the workspace after the person, not the address', () => { + expect(defaultWorkspaceName(undefined, 'bob@example.com')).toBe("Bob's workspace") + expect(defaultWorkspaceName(undefined, 'ada.lovelace@example.com')).toBe( + "Ada Lovelace's workspace" + ) + expect(defaultWorkspaceName(undefined, 'jean-luc_picard+wm@example.com')).toBe( + "Jean Luc Picard Wm's workspace" + ) + }) + + it('prefers the name the login provider gave', () => { + expect(defaultWorkspaceName('Ruben', 'r.k@example.com')).toBe("Ruben's workspace") + // Blank is not a name: fall back rather than produce "'s workspace". + expect(defaultWorkspaceName(' ', 'bob@example.com')).toBe("Bob's workspace") + }) + + it('falls back rather than offering a name the backend refuses', () => { + // Over the 50-char cap the field would be prefilled with something rejected on submit. + expect(defaultWorkspaceName('Bartholomew Maximilian Featherstonehaugh III', undefined)).toBe( + 'My workspace' + ) + // Nothing to derive from at all. + expect(defaultWorkspaceName(undefined, undefined)).toBe('My workspace') + expect(defaultWorkspaceName(undefined, '@example.com')).toBe('My workspace') + }) +}) + +describe('usernameFromName', () => { + // The `proper_username` constraint is `^[\w-]+$`, so a suggestion outside it is posted and + // then refused by the database, with the form showing nothing that explains why. + it('keeps only what the username constraint accepts', () => { + expect(usernameFromName("O'Connor")).toBe('oconnor') + expect(usernameFromName('alice+demo')).toBe('alicedemo') + expect(usernameFromName('Jean-Luc')).toBe('jean-luc') + expect(usernameFromName('ada.lovelace')).toBe('adalovelace') + }) + + it('answers undefined when nothing usable is left', () => { + // The caller opens the full form instead of prefilling something unusable. + expect(usernameFromName('++')).toBeUndefined() + expect(usernameFromName('')).toBeUndefined() + }) + + it('answers undefined rather than a value the column cannot hold', () => { + // `usr.username` is VARCHAR(50) while the name and email it is derived from run to 255, + // and `create_workspace` inserts it untruncated. + expect(usernameFromName('a'.repeat(50))).toBe('a'.repeat(50)) + expect(usernameFromName('a'.repeat(51))).toBeUndefined() + }) +}) + +describe('loadUsernamePolicy', () => { + // Neither default is safe — `create_workspace` refuses a username on an automating + // instance and requires one otherwise — so an unreadable setting has to reach the caller + // as a failure rather than as a guess it cannot tell apart from an answer. + it('rejects rather than guessing when the setting cannot be read', async () => { + getGlobal.mockRejectedValueOnce(new Error('502')) + await expect(loadUsernamePolicy()).rejects.toThrow('502') + }) + + it('automates when the setting says so, and when it is unset', async () => { + getGlobal.mockResolvedValueOnce(true) + expect(await loadUsernamePolicy()).toEqual({ automate: true }) + getGlobal.mockResolvedValueOnce(null) + expect(await loadUsernamePolicy()).toEqual({ automate: true }) + }) +}) diff --git a/frontend/src/lib/workspaceCreation.ts b/frontend/src/lib/workspaceCreation.ts index 49e4bdbea3..e4a9045f42 100644 --- a/frontend/src/lib/workspaceCreation.ts +++ b/frontend/src/lib/workspaceCreation.ts @@ -14,6 +14,7 @@ import { usersWorkspaceStore } from '$lib/stores' import { switchWorkspace } from '$lib/storeUtils' import { isCloudHosted } from '$lib/cloud' import { base } from '$lib/base' +import { WORKSPACE_NAME_MAX_LENGTH } from '$lib/utils/workspaceId' /** * Whether this user may create a workspace at all. Self-hosted instances default @@ -44,11 +45,34 @@ export interface UsernamePolicy { suggested?: string } +/** What `usr.username` holds, and neither the provider name nor the email is bounded by it. */ +const USERNAME_MAX_LENGTH = 50 + +/** + * A username the whole `usr.username` contract accepts: the `proper_username` constraint + * (`^[\w-]+$`, so word characters and hyphens and nothing else) and the column's own 50 + * characters. Anything outside the class is dropped rather than substituted — `O'Connor` is + * `oconnor`, not `o-connor`. + * + * Undefined where nothing usable is left or where what is left is too long, which is the + * caller's cue to ask for one: `create_workspace` inserts this value with no truncation, so a + * name the column refuses would fail on insert with nothing on screen naming the field. + */ +export function usernameFromName(name: string): string | undefined { + const cleaned = name.toLowerCase().replace(/[^\w-]/g, '') + return cleaned === '' || cleaned.length > USERNAME_MAX_LENGTH ? undefined : cleaned +} + /** * `createWorkspace` rejects a username when the instance automates them and * requires one when it does not, so the field only exists in the second case. */ export async function loadUsernamePolicy(): Promise { + // Rejects rather than defaulting when the setting cannot be read, because neither + // default is safe: `create_workspace` refuses a username on an instance that automates + // them and requires one on an instance that does not (`workspaces.rs:5820`). A caller + // that cannot read this cannot pick a request shape, and must say so instead of posting + // one of the two the server rejects. const automate = ((await SettingService.getGlobal({ key: 'automate_username_creation' @@ -57,7 +81,7 @@ export async function loadUsernamePolicy(): Promise { try { const me = await UserService.globalWhoami() const from = me.name ? me.name.split(' ')[0] : me.email.split('@')[0] - return { automate: false, suggested: from.replace(/\./g, '').toLowerCase() } + return { automate: false, suggested: usernameFromName(from) } } catch { return { automate: false } } @@ -78,3 +102,28 @@ export async function enterNewWorkspace(id: string): Promise { await refreshWorkspaceList() switchWorkspace(id) } + +/** + * How long a screen that hands over to a workspace stays up, whatever the server does. + * Creating or naming one takes a few hundred milliseconds, and a button that swaps the page in + * that time reads as nothing having happened — the floor is what makes it read as an action + * that ran, and it covers the workspace layout's first load on the other side. + */ +export const WORKSPACE_HANDOVER_MS = 900 + +/** + * What to call a workspace before its owner has said. The login provider's name when it gave + * one, else the email local part read as a name: `bob@…` is Bob, `ada.lovelace@…` is Ada + * Lovelace. Capped at what `create_workspace` accepts, since it is prefilled rather than + * typed and a name the server would reject must never appear in the field. + */ +export function defaultWorkspaceName(name: string | undefined, email: string | undefined): string { + const display = (name?.trim() || (email ?? '').split('@')[0]) + .split(/[._\-+\s]+/) + .filter(Boolean) + .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) + .join(' ') + .trim() + const proposed = display ? `${display}'s workspace` : 'My workspace' + return proposed.length > WORKSPACE_NAME_MAX_LENGTH ? 'My workspace' : proposed +} diff --git a/frontend/src/routes/(root)/(logged)/+layout.svelte b/frontend/src/routes/(root)/(logged)/+layout.svelte index e82d328c76..5aa90b80a2 100644 --- a/frontend/src/routes/(root)/(logged)/+layout.svelte +++ b/frontend/src/routes/(root)/(logged)/+layout.svelte @@ -32,6 +32,7 @@ type UserExt, defaultScripts, hubBaseUrlStore, + hubBaseUrlKnown, wsBaseUrlStore, disableHubStore, usedTriggerKinds, @@ -60,7 +61,6 @@ } from '$lib/components/sidebar/FavoriteMenu.svelte' import { SUPERADMIN_SETTINGS_HASH, USER_SETTINGS_HASH } from '$lib/components/sidebar/settings' import { isCloudHosted } from '$lib/cloud' - import { syncTutorialsTodos } from '$lib/tutorialUtils' import { PanelLeftClose, PanelLeftOpen, Home, Play, Search, WandSparkles } from 'lucide-svelte' import { getUserExt } from '$lib/user' import { confirmPendingLoginMethod } from '$lib/lastLoginMethod' @@ -468,7 +468,6 @@ function onLoad() { loadFavorites() - syncTutorialsTodos() loadHubBaseUrl() loadWsBaseUrl() loadDisableHub() @@ -476,10 +475,18 @@ } async function loadHubBaseUrl() { - $hubBaseUrlStore = - ((await SettingService.getGlobal({ key: 'hub_accessible_url' })) as string) || - ((await SettingService.getGlobal({ key: 'hub_base_url' })) as string) || - DEFAULT_HUB_BASE_URL + // A read that throws leaves the store on its seeded default, which names the public hub + // — so the flag, not the value, is what says the instance has answered. An instance that + // simply has no setting still answers: the chain falls through to the default. + try { + $hubBaseUrlStore = + ((await SettingService.getGlobal({ key: 'hub_accessible_url' })) as string) || + ((await SettingService.getGlobal({ key: 'hub_base_url' })) as string) || + DEFAULT_HUB_BASE_URL + $hubBaseUrlKnown = true + } catch (error) { + console.error('Could not read the hub URL:', error) + } } async function loadWsBaseUrl() { @@ -1107,7 +1114,7 @@