Skip to content

fix: full code apps deployable on merge UI and deploy UI#8451

Merged
rubenfiszel merged 9 commits intomainfrom
full-code-apps-merge-ui
Mar 19, 2026
Merged

fix: full code apps deployable on merge UI and deploy UI#8451
rubenfiszel merged 9 commits intomainfrom
full-code-apps-merge-ui

Conversation

@wendrul
Copy link
Contributor

@wendrul wendrul commented Mar 19, 2026

No description provided.

@claude
Copy link
Contributor

claude bot commented Mar 19, 2026

Claude finished @wendrul's task in 2m 19s —— View job


Review of PR #8451: Full code apps deployable on merge UI and deploy UI

  • Gather context and read the diff
  • Review code changes
  • Post review feedback

Summary

This PR fixes workspace cloning (clone_apps) so that app bundles (the bundled JS/CSS for full-code/raw apps) are carried over when cloning a workspace — which is what happens during merge and deploy operations. Previously, only app_version rows were cloned but their associated app_bundles rows were not, making full-code apps non-functional after a merge/deploy.

Code Quality

The implementation is clean and follows the existing patterns in the file well:

  1. Version ID mapping (version_id_mapping: HashMap<i64, i64>) — correctly tracks old-to-new version IDs by switching from .execute() to .fetch_one() with RETURNING id. Good approach.

  2. Bundle cloning — queries bundles by old version IDs and source workspace, then inserts them with the remapped version ID and target workspace. Follows the same pattern as other clone functions in the file.

  3. Guard clause (if !version_id_mapping.is_empty()) — avoids an unnecessary query when there are no versions. Nice touch.

Potential Issues

  1. No issues found with correctness — The SQL queries are properly parameterized, the mapping logic is sound, and the w_id is correctly set to target_workspace_id for cloned bundles.

  2. Ordering consideration — The bundle cloning happens after all versions are inserted but before app.versions arrays are updated (L3313-3323), which is the correct order since bundles reference app_version_id, not app.versions.

  3. clone_raw_apps redundancy?clone_raw_apps (L3363-3380) clones the raw_app table separately. This PR adds bundle cloning to clone_apps. Both paths seem necessary since raw_app and app_bundles are different tables serving different purposes — raw_app stores the source/metadata while app_bundles stores the compiled output. No issue here.

Verdict

Looks good. The change is minimal, well-scoped, and follows existing patterns. No bugs, security issues, or performance concerns.


How to verify

Create a full-code app (raw app) in a source workspace, then use the merge UI (workspace settings > deploy/merge) or the deploy UI to deploy it to a target workspace. After deployment, verify the full-code app loads and renders correctly in the target workspace — previously it would fail because the bundled assets were missing.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 19, 2026

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: d2a624c
Status:⚡️  Build in progress...

View logs

@wendrul
Copy link
Contributor Author

wendrul commented Mar 19, 2026

/update_sqlx

@wendrul
Copy link
Contributor Author

wendrul commented Mar 19, 2026

/update-sqlx

@wendrul wendrul marked this pull request as draft March 19, 2026 10:41
@wendrul wendrul marked this pull request as ready for review March 19, 2026 15:14
@claude
Copy link
Contributor

claude bot commented Mar 19, 2026

Claude finished @wendrul's task in 2m 29s —— View job


Review of PR #8451: Full code apps deployable on merge UI and deploy UI

  • Gather context and read the diff
  • Review backend changes
  • Review frontend changes
  • Post review feedback

Summary

This PR makes full-code (raw) apps work correctly through the merge UI and deploy UI by:

  1. Backend — Cloning app_bundles (compiled JS/CSS) when cloning a workspace, by tracking old→new app_version ID mappings
  2. Backend — Treating "raw_app" as a valid kind alongside "app" in compare_workspaces and query_visible_items
  3. Frontend — Adding "raw_app" to all kind-check conditions in the deploy/compare/on-behalf-of flows, plus adding a "Raw" badge for visual distinction

Code Quality

The implementation is clean, well-scoped, and follows existing patterns:

  • Version ID mapping (version_id_mapping: HashMap<i64, i64>) — correctly switches from .execute() to .fetch_one() with RETURNING id to track the mapping. Sound approach.
  • Bundle cloning — properly filters by both app_version_id and w_id, then inserts with the remapped version ID and target workspace. The guard clause (if !version_id_mapping.is_empty()) avoids unnecessary DB queries.
  • Frontend — all four files (CompareWorkspaces.svelte, DeployWorkspace.svelte, OnBehalfOfSelector.svelte, utils_workspace_deploy.ts) consistently add "raw_app" to existing "app" checks. The FileJson icon + "Raw" badge is a nice UX touch.

No Issues Found

The SQL is properly parameterized, the ordering is correct (bundles are cloned after versions but before app.versions arrays are updated), and the deployItem function already had raw_app handling via app.raw_app flag — the kind === 'raw_app' addition just ensures it reaches that code path.

The formatting-only changes in OnBehalfOfSelector.svelte and DeployWorkspace.svelte are harmless (likely auto-formatter).

Verdict

Looks good. Minimal, well-targeted fix. No bugs, security issues, or performance concerns.


Testing Instructions

Navigate to Workspace Settings > Content in a workspace that has a full-code (raw) app. Set up a staging/target workspace via the Deploy to or Merge feature. In the Compare Workspaces or Deploy Workspace dialog, verify that raw apps now appear in the diff list with a "Raw" badge. Deploy or merge a raw app to the target workspace, then open it there — it should load and render correctly (previously it would fail because the bundled JS/CSS assets were missing). Also verify the "on behalf of" selector appears for raw apps just like for regular apps.

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

Previous ee-repo-ref: 78d1f6cc4b15ec4c0768969635ba6b8f166a7742

New ee-repo-ref: b3b8005d45e3f2aa7228c61d2e4ae86a17d89a30

Automated by sync-ee-ref workflow.
@windmill-internal-app
Copy link
Contributor

🤖 Updated ee-repo-ref.txt to b3b8005d45e3f2aa7228c61d2e4ae86a17d89a30 after windmill-ee-private PR #470 was merged.

@windmill-internal-app windmill-internal-app bot changed the title [merge ee first] fix: full code apps deployable on merge UI and deploy UI fix: full code apps deployable on merge UI and deploy UI Mar 19, 2026
@rubenfiszel rubenfiszel merged commit 0e022b1 into main Mar 19, 2026
12 of 13 checks passed
@rubenfiszel rubenfiszel deleted the full-code-apps-merge-ui branch March 19, 2026 17:01
@github-actions github-actions bot locked and limited conversation to collaborators Mar 19, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants