4 Commits

Author SHA1 Message Date
Ruben Fiszel 81b23a2ba0 feat: make the fork lineage the only deploy relationship (#10410)
* feat: make the fork lineage the only deploy relationship

`workspace_settings.deploy_to` (2023) and `workspace.parent_workspace_id` (2025)
both expressed "which workspace does this one deploy into". Fork creation and
dev-workspace attach seeded both, but nothing kept them in agreement, so every
reader picked one and they disagreed.

Drop `deploy_to`. A migration folds surviving pairs into the lineage: a sole
claimant on a target with no dev workspace becomes that target's dev workspace
and keeps its own job tags, while many-to-one pairs become plain forks. Pairs
that the lineage cannot express -- dangling target, self-reference, chain,
mutual -- are reported and left unlinked.

Job tags were never lineage-aware: `per_workspace_tag` mapped any parented
workspace to its parent while `$workspace` interpolated the raw id, so a fork
running a script tagged `<tag>-$workspace` produced a tag no worker serves and
the job queued forever. Both paths now resolve to the nearest ancestor whose id
an admin would provision workers for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: preserve unconvertible deploy links and sweep tag caches on reparent

Review findings on the deploy_to unification:

- convert chains instead of discarding them, and keep whatever the lineage
  cannot express in workspace_deploy_to_unmigrated so the down migration can
  restore it
- ignore soft-deleted workspaces when choosing between a dev workspace and a
  plain fork; an archived claimant was demoting live pairs
- mirror attach_dev_workspace's git-sync strip, which the migration skipped
- sweep the tag cache over whole subtrees on rename and delete: tag resolution
  now walks ancestors, so a nested fork kept a tag nothing serves
- call a dev workspace a dev workspace in the settings copy
- redirect a root away from ?tab=deploy_to instead of rendering an empty target

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: detect lineage cycles and record archived links in the deploy_to migration

Second review round on the unification:

- detect cycles over the lineage as it would exist after conversion, not over
  the deploy_to graph alone: a root whose target was one of its own forks
  closed a loop that no deploy_to edge revealed
- record an archived source's link instead of filtering it out entirely, which
  dropped it with the column
- treat a fork whose deploy_to merely repeats its parent as redundant rather
  than reporting every pre-existing fork as unmigrated
- read the row count from the lineage update rather than the git-sync one
- sweep the tag cache when archiving a dev workspace, the last site that
  mutates is_dev_workspace without one

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: resolve $workspace on preprocessed flow tags regardless of $args

Third review round on the unification:

- a flow tag containing only `$workspace` skipped interpolation entirely on the
  preprocessed path, because the branch that ran it keys on `$args`. The raw
  tag was written back and named a queue no worker serves. Resolve `$workspace`
  before the branch and leave `$args` to it.
- record the new table's foreign key in the schema summary
- describe what the archive tag sweep actually does: the dev flag is cleared for
  any archived workspace, which is why it is unconditional

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: keep the deploy_to leftovers table only when it holds something

* fix: sweep tag caches on archive only where the dev flag actually changes

* feat: broadcast lineage changes and walk ws_specific ancestors only

- propagate tag-cache invalidation across processes over notify_events: the
  cache is per-process, so replicas kept resolving stale lineage for the TTL.
  The listener clears the whole cache rather than tracking ids, since a single
  mutation invalidates an unbounded set of descendants and lineage changes are
  rare admin actions.
- narrow list_ws_specific_versions to ancestors: walking down as well made a
  root fan out over its entire live fork subtree, and each member costs an
  identity lookup plus an RLS switch and probe. Ancestors are bounded by the
  fork depth limit.
- probe the leftovers table unqualified so rollback restores on a PG_SCHEMA
  install, where search_path is not public
- drop the nativets client method for the removed edit_deploy_to endpoint

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: let a prod see its dev workspace in ws_specific, and stop the walk oscillating

Descending into plain forks made a root fan out over its whole live fork
subtree, but a dev workspace is the paired editable environment rather than a
throwaway copy, so a prod should still see it. There is at most one per parent
and attach rejects nested dev chains, so that edge stays bounded.

The edges run both ways, so the recursion never converged: it bounced
parent<->dev until the depth cap on every call, 33 rows for a two-member set.
A visited-path guard ends the walk when nothing new is reachable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: keep dev pairings unnested, gate the delete broadcast, cover the ws_specific walk

Fifth review round:

- a root that already owns a dev workspace no longer converts: linking it under
  its deploy target would leave that dev nested beneath a fork, the shape
  attach_dev_workspace refuses to create. The link is preserved instead.
- broadcast a lineage change on delete only when descendants are orphaned.
  Deleting a leaf, which ephemeral fork churn does constantly, changes nobody
  else's resolution and was making every replica drop its whole tag cache.
- call list_ws_specific_versions in a test. plpgsql defers everything past a raw
  parse to the first call, so replaying the migration only proved it parses.
- use unwrap_or_default for the descendant sweeps, which run after the
  transaction has committed; a transient failure must not fail the request
- trim the traversal comment to the four-line limit

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: cache the renamed tally query and clear instance alerts on conversion

The integration test's query was never cached: `cargo sqlx prepare` without
--all-targets skips test targets entirely, and renaming its fixture workspace
changed the query text. Regenerated with --all-targets --features
all_sqlx_features,private, which is what lets the EE-gated otel test compile.

Also from review:
- clear error_handler_fallback_to_instance_alerts on converted workspaces.
  Dispatch ignores it once a parent exists, but the settings page keeps
  submitting the stored true, which the API rejects on a fork.
- restore the schema summary row to the file's name: columns format and put it
  back in alphabetical order

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: never cache an unresolvable tag workspace, and unadvertise the removed endpoint

- lookup_tag_workspace cached a "no row" result as self-resolution. A rename
  resolves the new id before its row lands, so a fork could be pinned to its own
  wm-fork-* id -- which nothing serves -- for the whole TTL, and its schedules
  kept re-pushing onto that dead tag. Fall back for the call without caching,
  matching how the error path already behaved.
- change_workspace_id swept its children but never itself. Sweep the new and old
  ids and broadcast unconditionally, since a rename always changes lineage.
- openapi-deref.{json,yaml} are served to clients via include_str!, so they were
  advertising edit_deploy_to after it started 404ing. The audit-action enum
  keeps the entry: historical rows still carry it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: align the served YAML spec with the JSON one and correct two comments

- the YAML deref lost the removed path but kept deploy_to on get_settings,
  so the two served specs disagreed. Both are now identical.
- the rename-sweep comment blamed cached-unresolvable lookups, which the same
  commit stopped caching. The real reason is that workspace ids are
  reclaimable, so a new id can carry a previous occupant's resolution.
- the instance-alert comment claimed the settings page submits the stored true
  and gets a 400. It hides the option on a fork and sends false; the hazard is
  the value outliving the pairing and re-enabling alerts after a detach.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore: update ee-repo-ref to 82da6cb2bafeda18acd6b70c599013a12117ecb0

This commit updates the EE repository reference after PR #694 was merged in windmill-ee-private.

Previous ee-repo-ref: f9ddf6a75aa13d1c13a3d7216a361a96f75ca435

New ee-repo-ref: 82da6cb2bafeda18acd6b70c599013a12117ecb0

Automated by sync-ee-ref workflow.

* fix: grant the deploy_to preservation table to the windmill roles

* test: drop the one-shot migration tests, keep the ws_specific execution guard

The two conversion tests replayed the migration against the fully-migrated
schema, which is not how it runs -- in production it runs mid-sequence against
the schema as of that point. A later migration touching workspace or
workspace_settings would break them without breaking anything real, and sqlx
checksums already freeze a released migration. They earned their keep finding
the archived-claimant and nested-dev cases during development; there is nothing
left for them to guard.

list_ws_specific_versions is different: it is live, no caller exercises it, and
plpgsql only parses a function body until first call.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: invalidate a reclaimed fork id cluster-wide without flushing every entry

Gating the delete broadcast on orphaned descendants stopped leaf churn flushing
every replica, but fork ids are reclaimable: the deleting process invalidated
locally while every other replica kept the old parent for the TTL, so a job
pushed in a recreated fork routed to the previous parent's tag.

The broadcast payload now carries meaning. A workspace id drops that one entry,
used for leaf deletion where exactly one id changed what it denotes. The `*`
sentinel drops everything, used for attach, detach, archive, rename and
deletions that orphan descendants -- reshaping a subtree no single id names.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs: name the right broadcast for each invalidation case

* docs: attach does invalidate the tag cache; the resolver walks the whole chain

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
2026-07-30 14:20:27 +00:00
Alexander Petric 95d4c6a94d feat(cli): non-interactive Slack connect/disconnect + sync round-trip fixes (#8935)
* feat(cli): non-interactive Slack connect/disconnect

Extract create_slack_workspace_artifacts / create_slack_instance_artifacts
from the browser OAuth callbacks and expose them via two new endpoints that
accept a pre-minted xoxb bot token:

- POST /w/{workspace}/workspaces/connect_slack (admin)
- POST /oauth/connect_slack_instance (super-admin)

Both produce bit-for-bit identical DB state to the UI browser flow.

Wire three CLI commands as thin wrappers:
- wmill workspace connect-slack
- wmill workspace disconnect-slack
- wmill instance connect-slack

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(cli): round-trip stability for workspace settings handlers

wmill sync push was destroying UI-configured error_handler/success_handler
state on every deploy. Two orthogonal bugs:

(a) pushWorkspaceSettings called editErrorHandler with `path: undefined`
    when the YAML lacked the handler block, which the backend treats as a
    clear — so syncing settings.yaml that didn't mention the handler wiped
    the DB row. Fix: skip the call entirely when absent from YAML.

(b) edit_error_handler omitted muted_on_cancel / muted_on_user_path when
    false, but the CLI always sends them, causing perpetual deepEqual
    drift and a spurious editErrorHandler call on every sync push. Fix:
    always persist both booleans.

migrateToGroupedFormat now preserves explicit `null` on
error_handler / success_handler as a "clear remote" signal distinct from
absence. Widen ErrorHandlerConfig | null / SuccessHandlerConfig | null to
make this explicit in the type.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(cli): sync support for workspace-level Slack OAuth override

Add slack_oauth_client_id and slack_oauth_client_secret to the v2 tarball
export and to pushWorkspaceSettings, so the workspace-level OAuth override
is now fully managed as code through settings.yaml.

Semantics:
  - both defined and truthy → setWorkspaceSlackOauthConfig (upsert)
  - both defined but falsy (e.g. empty strings) and remote has a value
    → deleteWorkspaceSlackOauthConfig
  - either omitted → leave remote alone ("not managed by git")

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(cli): normalize workspace settings sync to "omit = clear"

Earlier commits on this branch introduced an "omit = keep" rule for
error_handler / success_handler / slack_oauth_client_{id,secret} that
diverged from every other workspace setting (webhook, deploy_to, etc. all
treat YAML as canonical: absence = clear). Normalize:

- v2 tarball always emits these 4 fields (null when remote is NULL) so
  round-trip is bijective and settings.yaml is a complete snapshot.
- pushWorkspaceSettings drops the absent-from-YAML guards; YAML is
  canonical. Absence and explicit null both clear the remote — same rule
  as every other field.
- set_slack_oauth_config / delete_slack_oauth_config now fire
  handle_deployment_metadata so UI mutations reach git-sync-enabled
  workspaces' committed settings.yaml.

Policy for users: pull before push (same as every other setting). On first
post-upgrade pull, explicit `null` keys appear for any workspace whose
handlers / oauth override are unset — one-time YAML diff, no semantic
change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(cli): add unit + integration coverage for Slack settings sync

Unit tests (settings_unit.test.ts): cover migrateToGroupedFormat preserving
explicit `null` on error_handler / success_handler, and passthrough of
slack_oauth_client_id / _secret (both populated and null values).

Integration tests (slack_settings_sync.test.ts, skipped on CI per the same
convention as datatable_settings_sync.test.ts): exercise the full backend
via withTestBackend to verify

  1. pull emits null for unset error_handler / success_handler /
     slack_oauth_client_id / _secret;
  2. round-trip with all-null handlers is idempotent;
  3. push of populated slack_oauth_config upserts;
  4. omitting the slack_oauth keys from YAML clears remote (universal
     "omit = clear" rule);
  5. explicit null error_handler in YAML clears remote;
  6. round-trip preserves a populated error_handler exactly, including the
     always-persisted muted_on_cancel / muted_on_user_path booleans.

Also feature-gates `use crate::oauth2_oss::workspace_connect_slack` and its
route registration behind `cfg(feature = "oauth2")`: the import caused a
build failure on subsets of the workspace without the oauth2 feature,
surfaced by the integration test harness.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: bump ee-repo-ref to 59b6123

Pins windmill-ee-private to the tip of branch alp/slack_cli, which
contains the companion EE changes (helper extraction, non-interactive
Slack connect handlers, git-sync for Slack settings mutations).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Update SQLx metadata

* chore: regenerate system prompts for new slack CLI commands

Captures the new workspace connect-slack, workspace disconnect-slack,
and instance connect-slack commands in the auto-generated files that
CI enforces via system_prompts/check-freshness.sh.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: update ee-repo-ref to b4a5ca11e3b96ff03793c2bd396dbc1fe6ea1022

This commit updates the EE repository reference after PR #550 was merged in windmill-ee-private.

Previous ee-repo-ref: d7e44d0519327ec9077625130365e887826f324b

New ee-repo-ref: b4a5ca11e3b96ff03793c2bd396dbc1fe6ea1022

Automated by sync-ee-ref workflow.

---------

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>
Co-authored-by: Ruben Fiszel <ruben@windmill.dev>
2026-04-24 17:14:08 +00:00
Alexander Petric 39e77ecd00 feat: add slack connection fields to workspace settings export/import (#8287)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-09 21:39:03 +00:00
Ruben Fiszel a2cefdf0a2 refactor(cli): migrate CLI from Deno to Bun/Node.js (#8041)
* fix: only enable EE features in test backend when license key is available

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: skip EE tests without license key and exclude test-skills from test discovery

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: unskip passing tests and add duplicate (remote, workspaceId) check in addWorkspace

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* refactor(cli): migrate from Deno APIs to Node.js/Bun-compatible APIs

Replace Deno-specific APIs with Node.js equivalents across the entire CLI
codebase to enable running on Node.js/Bun. Switch build system from dnt
to bun, update imports from jsr:/npm: prefixed to bare specifiers, and
add package.json/tsconfig.json for the Node.js ecosystem.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* all

* test(cli): expand test coverage with new integration and unit tests

Add standalone_commands.test.ts covering folder list, schedule list,
resource-type list/push/update, script show/run/bootstrap, and user
commands. Add unit tests for filePathExtensionFromContentType and
removeExtensionToPath. Add git_unit, local_encryption_unit,
resource_folders_unit, and settings_unit test files. Fix schedule
cron expressions (6-field format), add includeSchedules flag, improve
test setup with pre-build and auto-cleanup, and support TEST_CLI_RUNTIME=node.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(cli): replace Deno.readFile with node:fs in WASM loaders and add schema parsing tests

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* refactor(cli): switch WASM parsers from local files to npm packages

Use published windmill-parser-wasm-* npm packages instead of local
wasm/ files. A loadParser() helper uses createRequire to resolve the
.wasm binary from node_modules and passes it to init() via
readFileSync, avoiding fetch() and Deno.readFile() patches.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test(cli): add coverage for --locks-required lint feature

Add 15 tests covering the lock-checking functionality merged from main:
- checkMissingLocks: standalone scripts (python, bun, bash), inline
  lock file resolution (valid, empty, missing), flow inline rawscripts
  (with/without locks, nested forloopflow), app inline scripts, raw
  apps without backend folder
- runLint --locks-required integration: reports issues when locks
  missing, skips checks when flag absent, passes when locks exist

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* ci(cli): replace Deno with Bun in CI workflows

- cli-tests.yml: remove Deno setup, use `bun test` instead of
  `deno test`, add `bun install` step for dependency installation
- npm_on_release.yml: replace Deno setup with Bun setup for CLI
  publishing
- build.sh: add `bun install` before building so CI has dependencies

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(cli): pre-start backend in test preload and remove Deno test leftovers

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(cli): normalize path separators for Windows compatibility

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* more tests + windows

* ci(cli): use Blacksmith runner for Windows tests

Switch test-windows job from windows-latest to blacksmith-16vcpu-windows-2025
for faster CI execution.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(cli): fix Windows path separator expectations in unit tests

buildMetadataPath and extractResourceName normalize to forward slashes
internally, so tests should not expect platform-specific separators in
their output.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(cli): fix Windows CI test failures for dev_server and script_run

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(cli): set BUN_PATH and NODE_BIN_PATH for backend worker on Windows

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* ci(cli): add SSH debug step on Windows test failure

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(cli): use native path separators for ignore check in dev mode on Windows

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-21 21:19:04 +00:00