From d3872cb18343b7cf08e520ebecc5246fe464d933 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Wed, 28 Oct 2026 06:07:55 +0100 Subject: [PATCH] feat: rebuild raw apps on a ui/-only change, fail sync push on ui/ errors (#12051) * feat: rebuild raw apps on a ui/-only change and fail sync push on ui/ errors Co-Authored-By: Claude Opus 5.5 * fix: catch unquoted css ui/ imports and surface app lookup errors Co-Authored-By: Claude Opus 5.5 * fix: match the raw app metadata file on windows sync paths Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- cli/src/commands/app/raw_apps.ts | 11 +++- cli/src/commands/shared_ui.ts | 78 ++++++++++++++++++++++++++++ cli/src/commands/sync/sync.ts | 52 ++++++++++++++++--- cli/test/shared_ui_diff_unit.test.ts | 52 ++++++++++++++++++- 4 files changed, 185 insertions(+), 8 deletions(-) diff --git a/cli/src/commands/app/raw_apps.ts b/cli/src/commands/app/raw_apps.ts index 570b03f949..194be441a7 100644 --- a/cli/src/commands/app/raw_apps.ts +++ b/cli/src/commands/app/raw_apps.ts @@ -35,6 +35,7 @@ import { import type { PermissionedAsContext } from "../../core/permissioned_as.ts"; import { buildPermissionedAsContext } from "../../core/permissioned_as.ts"; import { createBundle, detectFrameworks } from "./bundle.ts"; +import { importsSharedUi, sharedUiHash } from "../shared_ui.ts"; import { APP_BACKEND_FOLDER, RECORDINGS_FOLDER } from "./app_metadata.ts"; import { NEVER_DEPLOYED_DIRS, @@ -488,6 +489,12 @@ export async function pushRawApp( if (localApp.data) { value.data = localApp.data; } + // The bundle inlines ui/, so its content decides staleness as much as the + // app's own files do. Only the CLI reads this key back. + const uiHash = importsSharedUi(files) ? await sharedUiHash() : undefined; + if (uiHash) { + value.shared_ui_hash = uiHash; + } // extra_perms is synced independently via /acls/* — strip from the // up-to-date comparison so a perm-only edit doesn't trigger a rebuild + @@ -513,7 +520,9 @@ export async function pushRawApp( isSuperset({ ...localAppNoPerms, runnables }, app) && changedCustomPath(customPath, app) === undefined && isolationUpToDate(isolation, deployedPolicy, true); - const filesUpToDate = deepEqual(files, app.value?.files); + const filesUpToDate = + deepEqual(files, app.value?.files) && + app.value?.shared_ui_hash === value.shared_ui_hash; if (metadataUpToDate && filesUpToDate) { log.info(colors.green(`App ${remotePath} is up to date`)); } else { diff --git a/cli/src/commands/shared_ui.ts b/cli/src/commands/shared_ui.ts index f8e2965994..f12107fddb 100644 --- a/cli/src/commands/shared_ui.ts +++ b/cli/src/commands/shared_ui.ts @@ -1,9 +1,16 @@ import * as fs from "node:fs"; import * as path from "node:path"; +import { createHash } from "node:crypto"; import * as log from "../core/log.ts"; import { colors } from "@cliffy/ansi/colors"; import * as wmill from "../../gen/services.gen.ts"; import { readTextFile, readTextFileSync } from "../utils/utils.ts"; +import { + extractFolderPath, + extractResourceName, + isRawAppFolderMetadataFile, +} from "../utils/resource_folders.ts"; +import { APP_BACKEND_FOLDER } from "./app/app_metadata.ts"; const SHARED_UI_DIR = "ui"; @@ -26,6 +33,77 @@ async function readDirRecursive( return out; } +/** + * Whether an app's own files import from the shared `/ui/` folder. The `(` + * covers CSS's unquoted `@import url(/ui/theme.css)`. + */ +export function importsSharedUi(files: Record): boolean { + return Object.values(files).some((c) => /["'`(]\s*\/ui\//.test(c)); +} + +/** + * A digest of the local /ui/ folder, or undefined without one. An app + * bundles ui/ into itself, so this is what tells a push that the deployed + * bundle was built against other ui/ content than the local one. + */ +export async function sharedUiHash(): Promise { + const localDir = path.join(process.cwd(), SHARED_UI_DIR); + if (!fs.existsSync(localDir)) return undefined; + const files = await readDirRecursive(localDir); + const entries = Object.entries(files).sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)); + return createHash("sha256").update(JSON.stringify(entries)).digest("hex"); +} + +/** + * The `raw_app.yaml` keys of the local raw apps that import ui/ and whose + * deployed bundle was built from other ui/ content: no file of theirs changed, + * so the file diff never queues them. Apps in `skip` are already being pushed. + */ +export async function rawAppsStaleOnSharedUi( + workspace: string, + localMap: Record, + skip: Set, +): Promise { + const hash = await sharedUiHash(); + if (!hash) return []; + const apps = new Map< + string, + { metaPath?: string; files: Record } + >(); + for (const [key, content] of Object.entries(localMap)) { + const name = extractResourceName(key, "raw_app"); + if (!name || skip.has(name)) continue; + if (!apps.has(name)) apps.set(name, { files: {} }); + const app = apps.get(name)!; + const posix = key.replaceAll("\\", "/"); + const rel = posix.slice(extractFolderPath(key, "raw_app")!.length); + if (isRawAppFolderMetadataFile(posix)) { + app.metaPath = key; + } else if (!rel.startsWith(APP_BACKEND_FOLDER + "/")) { + app.files[key] = content; + } + } + const stale = await Promise.all( + [...apps].map(async ([name, { metaPath, files }]) => { + if (!metaPath || !importsSharedUi(files)) return undefined; + try { + const app = await wmill.getAppByPath({ + workspace, + path: name.replaceAll(path.sep, "/"), + }); + return (app.value as any)?.shared_ui_hash === hash + ? undefined + : metaPath; + } catch (e: any) { + // Not deployed yet: the file diff already queues it as added. + if (e?.status === 404) return undefined; + throw e; + } + }), + ); + return stale.filter((p): p is string => !!p); +} + /** The workspace's shared UI store, or undefined when it cannot be read. */ async function fetchSharedUi( workspace: string, diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 43bba850ed..cdf5a1ba1d 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -44,7 +44,12 @@ import { } from "../../types.ts"; import { downloadZip } from "./pull.ts"; import { runLint, printReport, checkMissingLocks } from "../lint/lint.ts"; -import { diffSharedUi, pullSharedUi, pushSharedUi } from "../shared_ui.ts"; +import { + diffSharedUi, + pullSharedUi, + pushSharedUi, + rawAppsStaleOnSharedUi, +} from "../shared_ui.ts"; import { pushMigrationFromDisk, offerToRunNewMigrations, @@ -5189,6 +5194,30 @@ export async function push( ); } + // A raw app inlines ui/ into its bundle, so ui/ content its deployed bundle + // was not built from makes it stale with no file of its own changed. + try { + const pushedApps = new Set( + changes + .map((c) => extractResourceName(c.path, "raw_app")) + .filter((n): n is string => !!n), + ); + for (const metaPath of await rawAppsStaleOnSharedUi( + workspace.workspaceId, + localMap, + pushedApps, + )) { + changes.push({ + name: "edited", + path: metaPath, + before: localMap[metaPath], + after: localMap[metaPath], + }); + } + } catch (e) { + log.warn(`Could not check raw apps against the shared UI folder: ${e}`); + } + // After the shared-lock pass, which reads a shared lockfile's deletion as the // signal that this checkout is not deduplicated — an advisory about the local // tree that holds whether or not remote items are being kept. @@ -6748,7 +6777,8 @@ export async function push( try { await pushSharedUi(workspace.workspaceId, opts.keepDeleted); } catch (e) { - log.warn(`Failed to push shared UI folder: ${e}`); + log.error(colors.red(`Failed to push shared UI folder: ${e}`)); + failedChanges.push({ path: "ui/", error: String(e) }); } try { await offerToRunNewMigrations( @@ -6784,7 +6814,9 @@ export async function push( ); } } - const pushedCount = changes.length - failedChanges.length; + // ui/ is pushed outside `changes`, so its failure is not one of them. + const pushedCount = + changes.length - failedChanges.filter((f) => f.path !== "ui/").length; if (opts.jsonOutput) { const result = { success: failedChanges.length === 0, @@ -6862,6 +6894,7 @@ export async function push( // Dry-run with no changes reaches here (a ui/ diff would have made changes // non-empty and returned above); never mutate the remote in that case. let sharedUiPushed = false; + let sharedUiError: string | undefined; if (!opts.dryRun) { try { sharedUiPushed = await pushSharedUi( @@ -6869,7 +6902,9 @@ export async function push( opts.keepDeleted, ); } catch (e) { - log.warn(`Failed to push shared UI folder: ${e}`); + sharedUiError = String(e); + log.error(colors.red(`Failed to push shared UI folder: ${e}`)); + process.exitCode = 1; } } // No changes pushed, so no new datatable migrations to run. @@ -6879,10 +6914,15 @@ export async function push( console.log( JSON.stringify( { - success: true, - message: sharedUiPushed + success: sharedUiError === undefined, + message: sharedUiError !== undefined + ? "Failed to push shared UI changes" + : sharedUiPushed ? "Pushed shared UI changes" : "No changes to push", + ...(sharedUiError !== undefined + ? { failed: [{ path: "ui/", error: sharedUiError }] } + : {}), total: 0, }, null, diff --git a/cli/test/shared_ui_diff_unit.test.ts b/cli/test/shared_ui_diff_unit.test.ts index 77328457e4..35c20770fa 100644 --- a/cli/test/shared_ui_diff_unit.test.ts +++ b/cli/test/shared_ui_diff_unit.test.ts @@ -14,6 +14,7 @@ import * as path from "node:path"; let remoteFiles: Record = {}; let remoteUnreadable = false; let pushedFiles: Record | undefined; +let deployedApps: Record }> = {}; mockServices({ getSharedUi: async (_args: { workspace: string }) => { @@ -26,9 +27,15 @@ mockServices({ }) => { pushedFiles = args.requestBody.files; }, + getAppByPath: async (args: { workspace: string; path: string }) => { + const app = deployedApps[args.path]; + if (!app) throw Object.assign(new Error("not found"), { status: 404 }); + return app; + }, }); -const { diffSharedUi, pushSharedUi } = await import( +const { diffSharedUi, pushSharedUi, rawAppsStaleOnSharedUi, sharedUiHash } = + await import( "../src/commands/shared_ui.ts" ); @@ -148,3 +155,46 @@ describe("pushSharedUi with keepDeleted", () => { .toEqual("keep me"); }); }); + +describe("rawAppsStaleOnSharedUi", () => { + let tmpDir: string; + let prevCwd: string; + + beforeEach(() => { + prevCwd = process.cwd(); + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "wm-shared-ui-apps-")); + process.chdir(tmpDir); + fs.mkdirSync(path.join(tmpDir, "ui"), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, "ui", "button.ts"), "v2", "utf-8"); + }); + + afterEach(() => { + process.chdir(prevCwd); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + const app = (name: string, index: string) => ({ + [path.join("f", `${name}.raw_app`, "raw_app.yaml")]: "summary: x", + [path.join("f", `${name}.raw_app`, "index.tsx")]: index, + }); + + test("queues an app importing ui/ only when its bundle was built from other ui/ content", async () => { + const hash = await sharedUiHash(); + deployedApps = { + "f/stale": { value: { shared_ui_hash: "old" } }, + "f/fresh": { value: { shared_ui_hash: hash } }, + "f/plain": { value: {} }, + "f/css": { value: {} }, + }; + const localMap = { + ...app("stale", `import { B } from "/ui/button"`), + ...app("fresh", `import { B } from "/ui/button"`), + ...app("plain", `import x from "./x"`), + ...app("css", `@import url(/ui/theme.css);`), + }; + expect(await rawAppsStaleOnSharedUi("ws", localMap, new Set())).toEqual([ + path.join("f", "stale.raw_app", "raw_app.yaml"), + path.join("f", "css.raw_app", "raw_app.yaml"), + ]); + }); +});