Same UserDraft.remove-without-clearing-in-memory bug as the previous
two commits, this time in the DiffDrawer's "Restore to draft" /
"Restore to deployed" buttons on all four /edit routes. The handler
deletes the DB draft (in the deployed case), wipes the localStorage
entry, navigates, and reloads — but the route's UserDraft handle
still holds the old draft + meta in memory, so the reload's
staleness check compares the stale meta against the freshly fetched
backend and surfaces a spurious "newer version was deployed" modal.
- scripts/edit, flows/edit, apps_raw/edit: route-level handle —
`handle.setDraftAndMeta(undefined, {})` before the reload.
- apps/edit: the handle lives in the AppEditor child, so force a
remount by setting `app = undefined; redraw++` before goto/loadApp
(matches the existing pattern from the toast's onResetToDeployed).
When the "Script/flow loaded from latest saved draft" toast's
"Reset to deployed" action ran, it:
1. Deleted the DB draft via DraftService.deleteDraft.
2. Called UserDraft.remove (clears localStorage only).
3. Called goto + loadScript / loadFlow.
But the handle's in-memory state still held the now-deleted DB draft
and its meta (remoteDraftRev pointing at the gone draft's created_at).
On the reload, the editor's loadScript/loadFlow saw `localDraft !=
undefined` and ran the staleness check, which compared
`meta.remoteDraftRev = <old timestamp>` against
`currentDraftRev = undefined`. Verdict: "version" stale → spurious
"A newer version was deployed on the server" modal, even though
nothing on the server actually moved. The editor visibly froze
behind the modal because the in-memory state wasn't refreshed.
Drop the in-memory state with `handle.setDraftAndMeta(undefined, {})`
before the reload — same fix already applied to the
"Restored from local storage > Reset to deployed" toast action.
apps/edit and apps_raw/edit's "discard draft" actions don't call
DraftService.deleteDraft (they just swap the in-memory view to the
deployed branch), so they don't hit this codepath.
The previous fix wrapped only the comparison reads in `untrack`, but
`handles.splice(0, handles.length, ...next)` still reads `.length`
under the effect's tracking scope — same feedback loop, same
`effect_update_depth_exceeded`.
Move the whole "compare + splice" block inside `untrack`. The
downstream notification on splice still fires (untrack suppresses
dependency subscriptions on the producer side, not write
notifications), so consumers of `handles` still re-render.
The reconcile effect read `handles.length` / `handles[i]` for the
"unchanged?" early-exit optimisation and then `handles.splice(...)`
to publish the new array. Reading `handles` inside the effect
registered it as a dependency; the subsequent splice re-fired the
effect; ad infinitum (Svelte threw
`effect_update_depth_exceeded`).
Wrap the comparison reads in `untrack` so the effect's only
tracked dependency stays `getSpecs()`. The splice still fires the
downstream readers of `handles` (the whole point of `useMany`'s
reactivity); it just doesn't re-enter its own producer.
Without a sweep, a heavy user accumulates one localStorage entry per
(workspace, kind, path) they ever touched. The pre-PR single-key
autosave self-capped at one entry per editor; this one needs an
explicit GC pass.
Mechanism:
- Stamp every persist with `lastWrittenAt: Date.now()`. Added at four
sites: `useLocalStorageValue`'s new `transformBeforePersist`
option (covers both setter and deep-mutation persists),
`UserDraft.save`'s no-handle fallback, `persistDirect` (force-meta
writes), and the legacy migration. Done at persist time, not in
`wrap()`, so deep mutations bump the clock too — `wrap()` runs only
on `.draft =` assignments, which would leave the timestamp stale for
bind-mutated editor sessions.
- `gcUserDrafts(maxAgeMs = 30d)` walks every `userdraft/w/...` key,
removes the ones older than the cutoff. Entries written before this
field existed (pre-PR or pre-this-commit) get backfilled with the
current time on first sweep so a 30-day clock starts fresh; the
alternative — sweeping on sight — would wipe work that the legacy
migration just rescued.
- Wired into the logged-in layout: runs once on mount and every 30 min
via `setInterval` (cleaned up in the effect's return).
Tests use `vi.setSystemTime` to drive the clock; assertions on the
stored payload now go through a `storedShape` helper that strips
`lastWrittenAt` before string-comparing, so the existing
`expect(...).toBe(wrapped(...))` style still reads cleanly. New tests
cover the sweep, the backfill behaviour, the default 30d window, and
a custom `maxAgeMs`.
The flows/apps/apps_raw `/edit` routes had a `?nodraft=true` handler
that just stripped the param from the URL via `afterNavigate` —
nothing behind it. The original pre-PR semantics (and what every
caller assumes) was "skip the localStorage autosave on this load."
Mirror the synchronous wipe pattern already in /add: when nodraft is
present, call `UserDraft.remove(kind, path)` and strip the flag from
the URL via `window.history.replaceState`, before the UserDraft handle
is created. The handle then reads an empty entry and the editor opens
on the backend version. A plain reload (no nodraft) restores the
autosave normally.
Removed the redundant `afterNavigate` blocks. Dropped the now-unused
`afterNavigate` import in all three; apps/edit still imports
`replaceState` (used downstream), so only that name stayed.
The previous commit landed `useMany` calling `useLocalStorageValue`
directly. That works for the `use()` 1-spec wrapper (whose getter is
untracked, so the reconcile `$effect` never re-runs), but for dynamic
specs (ResourceEditor / VariableEditor) it leaks the persist `$effect`
into the reconcile `$effect`'s scope — meaning the second spec change
would destroy the first entry's deep-mutation persist loop.
Wrap the `useLocalStorageValue` creation in `$effect.root` so the
entry's reactivity lives in its own scope. Stash the returned
disposer on the entry and invoke it when the refcount hits 0.
The vitest runtime's `$effect.root` returns its disposer but never
runs the callback (a test-env quirk, not a production behaviour).
Kept a documented fallback that calls `useLocalStorageValue` directly
when the callback doesn't populate `stateRef`. In tests that path
parents the persist `$effect` to the test scope and lives long
enough; in production `$effect.root` runs the callback synchronously
per the Svelte 5 spec and the fallback is unreachable.
Public surface change:
- New `UserDraft.useMany(getSpecs: () => UserDraftSpec<V>[])` returns a
reactive array of handles. The reconcile loop acquires entries for
added specs, releases entries for removed specs, and re-uses cached
handles for unchanged keys so caller-captured references stay stable.
- `UserDraft.use(kind, path, opts?)` becomes a 1-len wrapper around
`useMany`. The spec getter is `untrack`ed so reactive opts
(`$workspaceStore` etc.) are still captured-once — current `use()`
semantics unchanged.
- `UserDraftHandle.release()` and the `manualRelease` option are gone.
Component teardown is handled by a single internal `onDestroy` that
releases every entry `useMany` acquired.
ResourceEditor + VariableEditor migrated:
- Replaced `Record<ws, Handle>` + manual `ensureHandle`/`release` with
a `workspaceSpecs: $state<Array<{ws, defaultValue}>>` plus a
derived `Record<ws, Handle>` that pairs each ws with its parallel
handle from `useMany`. `ensureHandle(ws)` is now just a push to
the specs array; `VariableEditor.reset()` clears it. The reconcile
loop handles acquisition/release end-to-end.
Tests:
- Dropped the `manualRelease`/`release` test; the option no longer
exists.
- Added a `useMany` test asserting per-spec entries, isolated
workspace-scoped localStorage keys, and a single onDestroy
registration covering every acquired entry.
Implementation note: I tried wrapping `useLocalStorageValue` in
`$effect.root` to give the entry's `$state`/`$effect` an independent
scope (in case `useMany`'s reconcile effect tore down nested effects
across cycles). But `$effect.root`'s callback wasn't running
synchronously in the test runtime (vitest + svelte-vite plugin), and
the original `use()` implementation called `useLocalStorageValue`
directly without issue. Reverted to the direct call; the
nested-scope concern stays theoretical.
Three low-priority callouts:
- Document the variable.edited_at backfill in the migration. All
existing rows get a single `now()` timestamp from the column
DEFAULT; the staleness check only consumes the field as an opaque
rev string and never displays/sorts on it, so the collision is
harmless — but worth saying out loud.
- Add module headers to userDraftToast.ts and LocalDraftStaleModal.svelte
explaining how this layer sits above the per-browser UserDraft
autosave and is distinct from the backend DraftService (the
server-side "Save as draft" feature surfaced as `*.draft`).
None of the eight `new Triggers(...)` call sites pass an update
callback any more — the bridge was a leftover from the pre-UserDraft
era when ScriptBuilder ran its own localStorage autosave and had to
be notified on every triggers mutation. The unified UserDraft handle
now deep-tracks `script.draft_triggers` via the $effect in
ScriptBuilder, so the callback channel is dead weight.
Removes the third constructor parameter, the private field, and the
six `this.#updateDraftCallback?.()` invocations across setters and
mutators.
The four route-level editors (scripts/flows/apps/apps_raw) keep the
user's local draft visible behind the modal so they can glance at it
before choosing. The old body text described the situation (server
has moved on, local autosave is behind) but didn't say what's
actually on screen or how each action maps to it.
New body leads with "The editor is showing your local autosave" and
spells out each action: "Load latest replaces what's on screen; Keep
current leaves it alone." Same copy for both `cause = 'draft'` and
`cause = 'version'`, branching only on what the user is "behind"
relative to.
The original migration forced `USING created_at AT TIME ZONE 'UTC'`,
which tags every existing wall-clock value as UTC. That matches the
common case (Postgres on a UTC server, which the Docker image and most
managed offerings default to), but on a non-UTC operator's deployment
it shifts all pre-migration timestamps by the server's tz offset.
Drop the USING clause. Postgres's default `TIMESTAMP -> TIMESTAMPTZ`
cast reinterprets each existing value in the session's current
timezone — which is the same timezone under which the original
`INSERT ... DEFAULT now()` values were truncated to TIMESTAMP, so
the conversion correctly recovers the original instant regardless of
the operator's timezone. Same semantics on UTC servers, correct
semantics on non-UTC servers.
Down migration updated symmetrically.
The legacy migration was consuming any localStorage key starting with
`app-`, `flow-`, or `rawapp-`, with no constraint on what followed and
no shape check on the decoded payload. Two failure modes called out
in review:
1. A future feature (or third-party extension) picking a name like
`app-recent` would silently lose data on first migration run.
2. A stray key that happened to base64-decode to valid JSON but
wasn't a real legacy draft would still get promoted to the new
format, surfacing later as a phantom "Restored from local storage"
toast on the next edit.
Two guards:
- `LEGACY_PATH_SHAPE = /^[uf]\/[^/]+\/.+$/`: after a `<prefix>-` match,
the remainder must look like a Windmill item path (`u/owner/name`
or `f/folder/name`, possibly with deeper segments). Bare-prefix
empty-path entries (`app` / `flow` / `rawapp` for `/add` autosaves)
still match the exact branch and don't go through the shape gate.
- `isPlausibleLegacyValue`: after decode, require the payload to
carry the field the legacy writers actually produced
(`flow.flow` for flows, any of `summary|value|policy|path` for
apps, any of `files|runnables|data` for raw apps).
Both are belt-and-suspenders: nothing else currently uses these key
prefixes, but enforcing the shape locally keeps the migration safe
against future namespace collisions.
Adds `debounce: number` to `useLocalStorageValue`'s options. When set,
repeated mutations within the window collapse into a single
localStorage write fired by a plain `setTimeout`. The in-memory
`$state` is updated on every change so readers of `.val` always see
the latest value; only the persistence side-effect is deferred.
No `onDestroy` flush — the timer is independent of the Svelte
lifecycle, so SPA route teardown doesn't drop the pending write
(the callback still fires later as long as the JS context is alive).
A hard browser tab close within the window does drop it; that's an
acceptable trade-off vs the complexity of `beforeunload` listeners
and the leak/refcount issues they create alongside `useLocalStorageValue`'s
keyed instances.
`UserDraft.use` opts in with `debounce: 500` so a typing storm in the
script/flow/app editor produces one localStorage write per 500 ms
instead of one per keystroke.
Tests switch to `vi.useFakeTimers()` and a `flushPersist()` helper to
keep the synchronous `expect(localStorage…)` assertions working. New
test verifies the coalescing behaviour end-to-end.
The triggers in ScriptBuilder live in a dedicated `triggersState`
$state, separate from the `script` object that the UserDraft handle
deep-tracks. Pre-PR the per-builder localStorage autosave bridged the
two by snapshotting `triggersState.getDraftTriggersSnapshot()` into
the payload on every write — that bridge was dropped when we removed
the per-builder autosave in favour of UserDraft.
Add an $effect that deep-reads triggersState and mirrors the snapshot
back into `script.draft_triggers`. The UserDraft handle (already
deep-tracking `script`) then persists the trigger drafts as part of
the script autosave, restoring the prior behaviour.
The draft upsert was `ON CONFLICT (...) DO UPDATE SET value = EXCLUDED.value`,
so subsequent draft writes left `created_at` frozen at the first INSERT.
The frontend's UserDraft staleness check reads that timestamp as
`remoteDraftRev`; with it frozen, an updated remote draft looked
identical to the originally-baselined one and the "newer draft was
saved on the server" modal never fired.
Touch `created_at` on conflict too. The column's semantic widens from
"first write time" to "last write time", which is what every reader of
the field actually wants — the staleness signal is the only consumer.
SQLx offline cache regenerated to match the new query text.
ScriptBuilder's metadataOpen flag fires when `initialPath == ''` (the
heuristic for "new script, expected on /scripts/add"). The route's
`let initialPath = $state('')` left it empty until applyBaseline ran
later inside loadScript.
Pre-PR, the editor was gated on a route-level `script` $state that
started undefined, so ScriptBuilder didn't mount until loadScript's
synchronous block set both `script` and `initialPath` in the same
tick. With UserDraft.use reading localStorage synchronously, the gate
(`scriptHandle.draft`) is satisfied at mount time and ScriptBuilder
mounts with the still-empty initialPath, popping the drawer open.
Seed initialPath from page.params.path synchronously so ScriptBuilder
sees the path on its first render. Falls back to '' for the historical
`?hash=` view to preserve the existing behaviour there.
The new `*WithDraft` endpoints surface `draft.created_at` as
`Option<chrono::DateTime<Utc>>` for the frontend's staleness check,
which requires `TIMESTAMPTZ`. The column was originally created as
plain `TIMESTAMP`, so SQLx fails to deserialize any row that has a
non-null draft and the handler returns HTTP 400 instead of 200 —
caught by `test_draft_endpoints` in the integration tests.
Migrate the column to `TIMESTAMPTZ`, interpreting existing values as
UTC (matching `now()`'s behaviour on a UTC server). No compile-time
sqlx queries reference the column, so the offline cache stays valid.
* [ee] fix(nativets): pass tracing-enabled OtelConfig to deno_telemetry::init
* chore: update ee-repo-ref to 19a76a09ffb43649ee19e62d07e8b8a42d78757b
This commit updates the EE repository reference after PR #573 was merged in windmill-ee-private.
Previous ee-repo-ref: daf7ce31f192d64ca6b5602af626ab6453d2aff4
New ee-repo-ref: 19a76a09ffb43649ee19e62d07e8b8a42d78757b
Automated by sync-ee-ref workflow.
---------
Co-authored-by: windmill-internal-app[bot] <1429786+windmill-internal-app[bot]@users.noreply.github.com>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
* feat: include service accounts in instance settings users list
Service accounts (workspace-scoped, no password row) now appear in the
superadmin users list with a Bot icon, workspace badge, and a link to
manage them in the workspace settings. Role is locked to Operator.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: update sqlx offline cache
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: use composite key for users each block
Service accounts can share emails across workspaces, so key by email + workspace_id to avoid Svelte each_key_duplicate.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
UserDraft.remove only clears localStorage — the entry's reactive cell
stays alive as long as some component holds a handle. The toast
callback was relying on remove+loadXxx to reset state, but loadXxx
then read the *in-memory* autosave through the still-alive entry,
matched it against the now-deployed reference, and re-fired the same
toast. Forever.
Drop the in-memory state explicitly before the load:
- scripts/flows/apps_raw (route-level handle): `handle.setDraftAndMeta(undefined, {})`
- apps (handle lives in the AppEditor child): set `app = undefined`
to unmount AppEditor — its onDestroy releases the handle and the
entry's refcount drops to 0, destroying the entry.
ScriptBuilder / FlowBuilder / RawAppEditor briefly unmount while the
reload fetches; the flash is the user-visible "loading" cue.
Mirror the URL-hash seed mechanism from /scripts/add to /scripts/edit
for parity: decode the base64-JSON-encoded NewScript payload from the
URL hash, apply it over the bakedBaseline as the editor's initial
state, send "Loaded from URL", and strip the hash immediately via
window.history.replaceState so a reload restores the user's autosave
rather than re-injecting the seed.
The seed wins over local autosave + backend draft + deployed —
UserDraft.remove(script, draftPath) drops the stale autosave on disk
before setDraftAndMeta writes the seeded value, so the user's
subsequent edits will overwrite cleanly.
Skipped when ?hash= is in the URL (historical-version view, which is
read-only relative to drafts) and when the hash fragment isn't a
parseable encoded payload.
No callers build /scripts/edit#<encoded> URLs today — this lands the
mechanism for future symmetry with /scripts/add.
The URL-encoded script is a one-shot seed (Fork preview, workspace
handler templates, hub publish) — keeping the hash in the bar after
loading meant a reload would re-apply the original payload and wipe
whatever the user edited since landing.
After applying `urlScript` and firing the "Loaded from URL" toast,
clear `location.hash` via `window.history.replaceState`. The user's
edits then flow into the normal autosave path (UserDraft empty-path
entry), and a reload restores those edits instead of the seed.
The "Fork" action on run/[...run] and several workspace-settings
helper-script templates base64-JSON-encode a NewScript into the URL
hash on `/scripts/add#...`. Until now /scripts/add silently dropped
that payload — both call sites landed on a blank editor.
Decode `page.url.hash` at module top, and if it parses to an object,
apply it as `scriptHandle.draft` and surface "Loaded from URL". The
URL value wins over local autosave, ?template, ?hub, and YAML imports
because the hash represents an explicit "open this script" intent.
Parsing is inlined rather than reusing `decodeState` so an unrelated
hash (e.g. a future route anchor) doesn't fire its default "Impossible
to parse state" error toast.
When a route loads its local autosave (differs from backend, no
staleness alarm), surface "Restored from local storage" with up to
two reset actions:
- "Reset to saved draft": drop the autosave, reapply the backend DB
draft. Only shown when the backend has a DB draft.
- "Reset to deployed": drop the autosave, delete the DB draft on the
backend (if any), reload from the deployed version. Only shown when
the item has a deployed version.
The toast title + label wording + per-state inclusion live in a
single helper (`$lib/userDraftToast`). Each editor passes its own
reset callbacks since the side effects differ per route (handle vs
UserDraft.get/save, redraw counters, loadXxx helpers).
Wired to scripts/edit, flows/edit, apps/edit, apps_raw/edit. Resource
and variable editors don't have DB drafts and use per-workspace
handles — a follow-up will tailor a single-action version.
The previous cleanup ran in afterNavigate, which (a) fires asynchronously
— a quick reload between mount and the callback would re-wipe the
freshly-started draft — and (b) did `url.search = ''`, nuking sibling
params like ?template, ?hub, and ?wac.
Move the URL cleanup to the same synchronous block that calls
UserDraft.remove on nodraft, using `window.history.replaceState` so it
lands before paint. Only the `nodraft` key is removed — other params
survive.
Empty-path UserDraft entries persist now. Drop the leftover "in-memory
only" comments on the /add pages' handle creation, and rewrite the
EditorHeader save-initial-draft comments to describe why the UserDraft.remove
call is still needed: the draft was promoted to a real path on the
backend, so the prior-path autosave must not shadow a future "+ App" /
"+ Flow" / … visit.
We assume Windmill is the only app on the origin, so the
isPlausibleLegacyValue per-kind shape check was just dead weight.
Keep the cheap "decoded is an object" guard for malformed payloads.
* fix(python): preserve strings containing Infinity/NaN in result JSON
* test(python): add sanity checks for Infinity/NaN in results
* refactor(python): use string-aware regex callback for single-pass cleanup
* refactor(python): compact regex + handle backslash-escape parity
* perf(python): short-circuit cleanup when no Infinity/NaN/NUL in result
Empty paths used to be in-memory only (via the `isLocalOnly` short-circuit)
because we worried about collisions between concurrent /add tabs. The user
asked for the trade-off to flip: a /flows/add or /scripts/add reload should
restore the user's work, while explicitly clicking "+ Flow / + Script / …"
should always open a clean editor.
- Drop `isLocalOnly` from UserDraft so empty-path entries persist under
`userdraft/w/{ws}/{kind}/` like any other path. The existing per-kind
refcounting and saveInitialValue=false behavior already handle them
correctly — the change is just lifting the bypass.
- Each /add page now calls `UserDraft.remove(kind, '')` synchronously
when `?nodraft=true` is present in the URL, before the handle is
created.
- The two "+" entry points that lacked the `?nodraft=true` flag
(CreateActionsScript's plain `<a href>` and CreateActionsFlow's
YAML/JSON import paths) now include it, so every fresh-start path goes
through the wipe.
- Tests updated: the "empty path (in-memory only)" block becomes
"empty path (persists across reloads)" and asserts the new behavior.
* feat: read-only flag on API tokens, orthogonal to scopes
Add a per-token `read_only` boolean set at creation time. When true, the
token can only call HTTP methods classified as Read (GET/HEAD/OPTIONS).
Mutating methods and job-run actions are rejected with 403, regardless of
which scopes are attached. Surfaced as a prominent toggle in the standard
token-creation flow and a discreet `2xs` toggle in MCP mode (where users
often want write access, so we don't bias them toward enabling it).
MCP enforcement: read-only tokens hide all script/flow/hub tools from
`list_tools` and only see endpoint tools whose method is GET, and the
runner rejects `call_tool` on anything mutating.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: review fixes for read-only token flag
- Exempt /api/mcp/* and /mcp/* paths from the read-only middleware check.
MCP transport runs over POST (streamable HTTP / SSE), so otherwise the
middleware would 403 every MCP request before the runner could enforce
read-only at the tool-call level.
- Tighten is_endpoint_read_only to GET only, matching the read_only_hint
that create_endpoint_annotations actually emits.
- Add unit test for check_read_only_for_route covering GET/HEAD/OPTIONS,
mutating methods, and run paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: bump ee-repo-ref to read-only-trigger-toggle
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(frontend): make read-only toggle discreet in both modes
Match the MCP-mode treatment in standard mode: text-tertiary, 2xs, shared
"Read-only" label. The tooltip switches per mode so the explanation still
fits the context.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(frontend): gate read-only toggle behind Limit token permissions
The read-only toggle now only shows when the user has limited the token's
scopes (standard mode) or in MCP mode (which always picks an MCP scope).
Turning the limit off also resets read-only so it doesn't silently stick.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(frontend): hide incompatible MCP tools when read-only is on
When the read-only toggle is on in MCP mode:
- Endpoint badges and the custom-mode endpoint MultiSelect filter to GET.
- Already-selected non-GET endpoints are pruned from the scope.
- The scripts/flows preview is replaced with a note explaining they're
hidden (the runner already rejects script/flow runs for read-only).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(frontend): place read-only toggle at top of limited scope area
The previous gate required at least one scope to be picked before the
read-only toggle appeared, which made it look missing while the user was
still building their scope list. Move the toggle inside ScopesPicker:
- Standard mode: sits directly under the "Limit token permissions" toggle
whenever Limit is on, before the scope selector.
- MCP mode: sits at the top of the MCP scope block.
readOnly is now $bindable on ScopesPicker so CreateToken still owns the
value. The auto-reset on un-limit moves into ScopesPicker too.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(frontend): nest read-only toggle inside the scope list card
Place the read-only toggle at the top of the scope list (between the
Selected Scopes summary and the bordered domain list) via a new optional
topSlot snippet on ScopeSelector. Keeps ScopeSelector decoupled from
read-only specifics; ScopesPicker fills the slot.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: update ee-repo-ref to 9bc8160be50b3e57a60daf4e1b71c389a6e02b8a
This commit updates the EE repository reference after PR #571 was merged in windmill-ee-private.
Previous ee-repo-ref: f53d26e6685dfd60bfa67686fbd7358169cfd130
New ee-repo-ref: 9bc8160be50b3e57a60daf4e1b71c389a6e02b8a
Automated by sync-ee-ref workflow.
* fix: address CI review for read-only token flag
- P1 (Codex): narrow the MCP middleware exemption from "any /api/mcp/*"
to just the streamable HTTP transport endpoints
(/api/mcp/gateway, /api/mcp/w/{ws}/{mcp,sse,list_tools}). Without this,
a read-only token could POST /api/mcp/gateway/oauth/server/approve and
mint a follow-on non-read-only MCP token via the OAuth code/token
exchange.
- P2 (Claude/cubic): fix test comment/assertion mismatch — the run-path
assertion now exercises GET (which is what the RUN_PATH_ACTIONS
elevation comment describes) in addition to POST. Add a regression
assertion for /api/mcp/gateway/oauth/server/approve.
- P2 (cubic): short-circuit script/flow/hub-script/resource fetches in
MCP list_tools when read_only is on — they would only be discarded
below, so skipping the DB and resource fan-out is pure win.
- P2 (cubic): when scopes are pre-supplied via the CreateToken prop, the
ScopesPicker isn't rendered, which previously hid the read-only
toggle entirely. Render it next to the pre-supplied scopes display.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
- exists_script_by_path now filters archived = false, matching the
conflict check in create_script_internal. Previously the frontend
blocked creating a new script at a path occupied only by archived
scripts, even though renaming to that same path was allowed.
- Hide the Delete entry in the script details "..." menu unless the
user is admin. The backend delete_script_by_hash already requires
admin, so non-admins would always see an error after clicking.
* perf(dynselect): only retrigger when helper-script args actually change
Parse the inline helper's signature with the existing WASM parser and
restrict the form-arg diff to keys the helper actually consumes. Typing
into unrelated fields no longer queues a dynselect job every second.
Falls back to the previous full-args comparison when the helper is
deployed or parsing fails.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dynselect): avoid double helper-script fetch on mount
usePromise defaults to loadInit=true, so refresh() ran before the
JobLoader child was bound (firing a no-op pending promise) and the
$effect then fired a second refresh once the bind:this resolved.
Disable loadInit so the effect owns the single first call.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(dynselect): use parser directly instead of inferArgs
inferArgs mutates a Schema object we never use and goes through a
shared cache; when fed an empty schema for non-main entrypoints the
caller cannot reliably read back the resulting properties. Add
parseEntrypointArgs that just runs the parser and returns the
parameter name Set (or undefined when unknown / unsupported / has
rest args / function not found). DynamicInput uses that and keeps
the previous params in flight while the next parse is computing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynselect): support deployed helpers in smart retrigger
Add getHelperEntrypointArgs which dispatches on HelperScript.source:
inline parses immediately; deployed fetches the script (or the flow's
inline dyn-select code) once and caches per (workspace, kind, path,
entrypoint). Without this the /scripts/get/* run view fell back to
the full-args comparison and still retriggered on unrelated fields.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dynselect): zero-arg helpers report empty deps, not unknown
Codex review flagged that a valid zero-parameter entrypoint was being
treated as "couldn't determine signature" and falling back to the
full-args comparison. Distinguish "function found with no params" from
"function not found" via the parser's auto_kind field — only the
latter sets it, so empty args + auto_kind=null means a real zero-arg
helper and we return an empty Set (no retrigger on unrelated fields).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ensureHandle was doing a post-hoc `if (h.draft === undefined) h.draft = baseline`,
which relies on the saveInitialValue=false skip to swallow that seeding
write. Hand the baseline to `UserDraft.use({ defaultValue })` instead —
useLocalStorageValue uses it as the initial $state value when localStorage
is empty, so lastSerialized is correct out of the gate and no setter call
is needed.
Earlier commits in this PR wired the resource and variable editors to a
single multi-workspace bundle stored under the user's session workspace
key — which mixed workspaces in one localStorage entry and required a
custom multi-key fix-up pass to persist edits for other workspaces.
Reset both editors to their pre-PR shape and apply the minimal change:
the per-workspace `Record<string, ResourceState>` (resp. `VariableState`)
becomes `Record<string, UserDraftHandle<…>>`, with one handle per
workspace created via `UserDraft.use(…, { workspace: ws })`. The handle
keys its own localStorage entry under that workspace, so cross-workspace
edits stay cleanly separated and reactivity flows through the handle's
`draft` accessor — `bind:` on form fields just works.
Adds `manualRelease: true` + `handle.release()` to `UserDraft.use` so
the editors can register handles lazily inside an effect (Svelte 5
forbids `onDestroy` outside component init). The editors register a
single top-level `onDestroy` that releases every collected handle.
After a successful save, the per-workspace autosave is cleared via
`UserDraft.remove(itemKind, path, { workspace })`.
* fix(bun): pass --preserve-symlinks on unbundled execution
Bun 1.2/1.3 moved its global package cache to a content-addressed
layout and the installer now creates a single directory symlink from
node_modules/<pkg> to the cache entry. Without --preserve-symlinks,
Bun resolves modules from each file's realpath, so any require/import
inside an installed package walks up from cache_nomount/bun/... and
never finds the sibling deps living under <job_dir>/node_modules/.
This manifested as e.g.
ENOENT while resolving package 'zod/v3' from
'/tmp/windmill/cache_nomount/bun/@langchain/core@1.1.44@@@1/dist/...'
on //nobundling scripts that pull @langchain/core, even though zod is
correctly installed alongside it in node_modules.
The bundled execution path already had --preserve-symlinks since #4132
(needed because we symlink the cached bundle file into the job dir).
The unbundled path didn't, because at the time Bun installed via per-
file hardlinks and the realpath of node_modules entries was the job
dir itself. The Bun installer's layout change made the flag necessary
on the unbundled path as well.
Add the flag to all three unbundled `bun run` invocations:
- nsjail unbundled path
- non-nsjail unbundled path
- dedicated worker (always unbundled)
This also fixes a latent bug on the first run of any bun script that
imports a package whose internals reference siblings (the build_cache
path runs unbundled this round while it builds the bundle for next
time).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(bun): regression test for nobundling + transitive require resolution
Adds an integration test that mirrors the original failure: a //nobundling
script importing @langchain/core, which (in its CJS internals) does
require('zod/v3'). Before --preserve-symlinks was added to the unbundled
bun run invocations, this failed with:
ENOENT while resolving package 'zod/v3' from
'.../cache_nomount/bun/@langchain/core@<ver>@@@1/dist/runnables/base.js'
The test covers the non-nsjail unbundled path. Reproducibility of the
pre-fix failure depends on Bun's installer choosing the directory-symlink
layout for the node_modules entry (the default on Bun 1.2/1.3+ with the
new content-addressed global cache that produced the user's error).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Diego Imbert
Ruben Fiszel
