fix(cli): preserve user drafts on sync push and permissioned-as

[hugocasa]

Summary

wmill sync push (and the set-permissioned-as commands) deploy scripts/flows/apps through the same create/update API endpoints the UI "Deploy" button uses. Every one of those deploy paths runs DELETE FROM draft for the item's path inside the transaction. That's correct for a UI deploy-from-draft (the draft becomes the deployed version, so the "unsaved changes" marker is cleared) — but when a CLI/git-sync push touches a path, it silently destroys whatever in-progress draft a teammate has there, even though the push had nothing to do with that draft.

This fix makes CLI deploys preserve drafts while leaving UI deploys unchanged.

Approach

Mirror the existing deployment_message pattern: a transient, deploy-control body field skip_draft_deletion that the CLI sets to true on its deploy paths. When set, the backend skips the DELETE FROM draft; the deploy itself proceeds normally. UI deploys never send it (defaults to false), so their behavior is identical to before.

Changes

Backend

• Added skip_draft_deletion: Option<bool> to NewScript, NewFlow, CreateApp, EditApp (#[serde(default, skip_serializing_if = "Option::is_none")]).
• Guarded the 6 deploy-path DELETE FROM draft sites: create_script_internal (both parent/no-parent branches), create_flow, update_flow, create_app_internal, update_app_internal. The script snapshot and raw-app endpoints route through these same internal functions, so they inherit the guard.
• Deliberately excluded the flag from the NewScript Hash impl (it must not affect version identity) and bound it to _ in the no-op drift-guard destructure (caller intent, not script state).
• The dedicated draft-delete endpoint and entity-deletion (delete_script/delete_flow/delete_app, bulk delete) draft deletions are left untouched.

CLI

• Sets skip_draft_deletion: true on the sync push helpers (createScript, pushFlow, pushApp, pushRawApp) and on the set-permissioned-as redeploys for script/flow/app.

openapi.yaml

• Added the field to the 6 request schemas + the NewScript component. cli/gen and frontend/src/lib/gen are gitignored and regenerated from the spec at build time.

Test plan

Verified end-to-end against a running backend (script, flow, and app each with a saved draft):

• Push with skip_draft_deletion=true → draft survives (count 1), deploy succeeds
• Push without the flag (legacy/UI path) → draft deleted (count 0)
• Backend compiles and serves; CLI typecheck shows no new errors
• Manual UI check: "deploy from draft" in the editor still clears the draft (default-off path unchanged)
• Exercise a bundled/codebase script (snapshot endpoint) and a raw app via wmill sync push

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	b21cde4
Status:	✅  Deploy successful!
Preview URL:	https://95fd2e37.windmill.pages.dev
Branch Preview URL:	https://fix-sync-draft-override.windmill.pages.dev

View logs

[claude]

Claude finished @hugocasa's task in 5m 37s —— View job

---

cc @hugocasa

Mergeable, but should ideally address nits: no automated regression test for the draft‑preservation behavior; CLI scope is slightly broader than the PR title (every script/flow/app push, not only sync push) — confirm that's intended.

Summary

The fix is well‑scoped and correct. I verified the completeness and routing claims in the description against the actual code — they all hold up.

Completeness of DELETE FROM draft guarding — there are 14 DELETE FROM draft sites in the backend. The 6 deploy‑path sites are now correctly guarded:

• create_script_internal parent branch (scripts.rs:1368) and no‑parent branch (scripts.rs:1455) ✓
• create_flow (flows.rs:565) and update_flow (flows.rs:1168) ✓
• create_app_internal (apps.rs:1355) and update_app_internal (apps.rs:1964) ✓

The 8 untouched sites are all correctly left alone — I read each one: delete_app (apps.rs:1584), delete_script (scripts.rs:2719), delete_flow (flows.rs:1775) are entity‑deletion paths (with trashbin capture); plus the dedicated draft endpoint (drafts.rs:106), bulk delete (scripts.rs:2864), and the user‑offboarding / workspace‑wipe paths. None are deploys.

Routing claims verified:

• The snapshot endpoint (create_snapshot_script, scripts.rs:498) deserializes into NewScript and calls create_script_internal, so it inherits the guard. On the CLI side, createScript mutates body before branching, and the bundle branch reuses that same body (script.ts:801 form.append("script", JSON.stringify(body))) — so bundled/codebase pushes are covered even though the test‑plan item is unchecked.
• The raw‑app endpoints (create_app_raw/update_app_raw) route through create_app_internal/update_app_internal via process_app_multipart!, which deserializes the app field into CreateApp/EditApp, so the flag flows through.

Correctness of the exclusions:

• skip_draft_deletion is excluded from impl Hash for NewScript → version hash unaffected ✓
• It's bound to _ in the exhaustive destructure in is_noop_deploy_against_parent → no‑op detection unaffected, and the exhaustive pattern (no ..) means a future field addition forces a conscious decision ✓
• openapi createScript references #/components/schemas/NewScript, and the flow/app inline schemas are well‑formed, so the regenerated clients pick up the field ✓

No coverage gap for variables/resources: only script/flow/app drafts are ever deleted on a deploy path (typ is only ever those three in deploy DELETEs), so there was never a draft to wipe for other entity kinds.

Findings

P2 — CLI scope is broader than the PR title. The flag is set in the shared createScript helper (script.ts:763), which backs both wmill sync push and the single‑item wmill script push (callers at script.ts:570/587). Likewise pushFlow/pushApp back both flows. This is arguably the correct behavior — any CLI deploy‑from‑files should never wipe a teammate's in‑editor draft — but it's wider than "sync push and permissioned‑as." Just confirm this is intended; if so, no change needed.

No P0 or P1 issues found.

AGENTS.md compliance

No violations. The change follows the existing deployment_message / preserve_on_behalf_of transient‑flag pattern, keeps changes focused, and the doc comments explain the non‑obvious "why" (hash/no‑op exclusion) rather than restating the "what" — consistent with the repo's commenting guidance.

Test coverage

This is a backend behavior change to DB‑mutating API handlers. Per the repo's review policy, modified API handlers / DB access should ideally have integration tests. The only test change here (dependency_map.rs) just adds skip_draft_deletion: None to keep existing constructions compiling — there's no test asserting the new behavior (draft survives with the flag, draft deleted without it).

There's a clean pattern to mirror: backend/tests/preserve_on_behalf_of.rs already tests an analogous transient deploy flag, and the API‑client NewScript/NewFlow structs already expose skip_draft_deletion (you set None on them in dependency_map.rs), so a test could INSERT INTO draft, deploy with Some(true) vs unset, and assert the draft count. Recommended but not blocking.

Frontend: openapi regen touches frontend/src/lib/gen, but the UI never sends the flag (defaults to false), so no component test is expected.

Manual verification still worth doing before merge (the two unchecked test‑plan boxes): in the editor, perform a UI "deploy from draft" for a script/flow/app and confirm the unsaved‑changes marker still clears (the default‑off path must be unchanged); and run a wmill sync push that touches a path where another user has a saved draft and confirm the draft survives while the deploy still lands a new version.

[cubic-dev-ai]

No issues found across 12 files

_{Re-trigger cubic}

[github-actions]

Pi Review

Good to merge

---

Summary

This PR adds a skip_draft_deletion flag to the create/update request types for scripts, flows, and apps, allowing CLI/git-sync deploys (and set-permissioned-as redeploys) to preserve in-progress user drafts at the same path. UI deploys continue to clear drafts as before.

Findings

No blocking issues or nits identified.

Mechanics walkthrough

• Six deploy-path DELETE FROM draft sites guarded — create_script_internal (parent and no-parent branches), create_flow, update_flow, create_app_internal, update_app_internal. All six correctly use if !skip_draft_deletion / if !nf.skip_draft_deletion.unwrap_or(false) before executing the delete.

• Intentionally unguarded sites are correct — the delete-entity paths (delete_script, delete_flow, delete_app, bulk delete), the dedicated /drafts DELETE endpoint, workspace deletion, and user offboarding all delete drafts unconditionally. When an entity is being deleted, its draft should go with it. Left untouched as the PR body states.

• Hash exclusion is correct — skip_draft_deletion is deliberately omitted from impl Hash for NewScript (confirmed via the custom hash impl and the comment on L554). hash_script(&ns) won't produce a false version bump for a CLI re-push with the flag set. The field is also bound to _ in is_noop_deploy_against_parent's destructure, so it can't cause false no-op determinations.

• Serde defaults protect all callers — #[serde(default, skip_serializing_if = "Option::is_none")] on all four structs (NewScript, NewFlow, CreateApp, EditApp) means UI deploys never send it, backend sees None → unwrap_or(false) → draft deleted. CLI sends true → draft preserved.

• CLI coverage is complete — the flag is set to true in createScript, pushFlow (create + update), pushApp + pushRawApp (create + update), and all three set-permissioned-as handlers (script, flow, app).

• clone_script correctly sets skip_draft_deletion: None (intentional clone should delete the draft at the target path).

• dependency_map.rs test received the necessary struct field additions (skip_draft_deletion: None in two EditApp constructions).

• openapi.yaml has the field documented on all 6 request schemas + the NewScript component schema.

No false positives from sqlx::FromRow

NewFlow derives sqlx::FromRow (line 94 in flows.rs), but it is never used with query_as — it's exclusively a request-body type. The derive is benign.

Test coverage

• Backend: The change is a boolean guard on existing code. No new Rust unit or integration tests for this specific flag, but the risk is low — the guard is mechanical. The existing dependency_map.rs integration test was updated with the new struct field (None).
• CLI: No new tests. The CLI change adds a literal true key to existing request bodies — mechanical and low-risk.
• openapi.yaml: Schema-only change with no automation to test against.

Manual verification still needed

• Deploy a script from the UI draft editor ("Deploy" button) and confirm the draft row is deleted — this exercises the default-off path to ensure the UI behavior is unchanged from before this PR.
• Run wmill sync push with a script, flow, and app that each have a saved draft in the workspace. Verify each draft still exists after the push, and the deploy itself succeeded.

[github-actions]

Codex Review

Good to merge

No issues found. Checked the backend deploy paths, CLI call sites, OpenAPI shape, and AGENTS.md compliance.

Test coverage

Backend changes affect API deploy handlers and DB draft deletion behavior. I do not see an automated regression/integration test for preserving drafts on CLI deploys, so the manual scenarios in the PR body remain the main coverage.

Manual verification should still cover UI deploy-from-draft for script, flow, and app: deploying from the editor without skip_draft_deletion should clear the draft. It should also cover raw app and bundled/codebase script CLI pushes, confirming the deploy succeeds while an existing draft row remains.