From ff7d35682f4359a0cbb9a0939c329a0aa5b7f306 Mon Sep 17 00:00:00 2001 From: Adam Eivy Date: Wed, 2 Sep 2026 06:30:21 +0000 Subject: [PATCH] fix: rebuild production UI during managed app updates Managed Update app pulled, installed, and restarted without building client/dist. For PortOS that left the install out of sync and required a second Reconcile just to rebuild the UI. --- .../apps/tabs/RepositorySourcePanel.jsx | 1 + .../apps/tabs/RepositorySourcePanel.test.jsx | 1 + server/services/appUpdater.js | 36 +++++++--- server/services/appUpdater.test.js | 71 ++++++++++++++++++- 4 files changed, 99 insertions(+), 10 deletions(-) diff --git a/client/src/components/apps/tabs/RepositorySourcePanel.jsx b/client/src/components/apps/tabs/RepositorySourcePanel.jsx index 97622d45f9..15fc75a2e4 100644 --- a/client/src/components/apps/tabs/RepositorySourcePanel.jsx +++ b/client/src/components/apps/tabs/RepositorySourcePanel.jsx @@ -342,6 +342,7 @@ export default function RepositorySourcePanel({ appId, appName, onUpdated, refre
  • Pull the application checkout from its configured origin
    • {companions.length > 0 &&
  • Pull {companions.length} independent companion checkout{companions.length === 1 ? '' : 's'}
    • }
  • Install dependencies and run the app's setup script when configured
    • +
  • Rebuild the production UI when a build script is configured
    • {status?.updateRestartsApp &&
  • Restart the app's managed processes
    • } diff --git a/client/src/components/apps/tabs/RepositorySourcePanel.test.jsx b/client/src/components/apps/tabs/RepositorySourcePanel.test.jsx index e5d84b4e3e..cef4464cac 100644 --- a/client/src/components/apps/tabs/RepositorySourcePanel.test.jsx +++ b/client/src/components/apps/tabs/RepositorySourcePanel.test.jsx @@ -148,6 +148,7 @@ describe('managed app repository sources', () => { fireEvent.click(await screen.findByRole('button', { name: 'Sync fork & update app' })); const dialog = await screen.findByRole('dialog'); expect(dialog).toHaveTextContent('Pull 1 independent companion checkout'); + expect(dialog).toHaveTextContent('Rebuild the production UI when a build script is configured'); expect(dialog).toHaveTextContent('Restart the app\'s managed processes'); fireEvent.click(screen.getByRole('button', { name: 'Sync fork and update' })); diff --git a/server/services/appUpdater.js b/server/services/appUpdater.js index ccc854d265..0b67ea1cd6 100644 --- a/server/services/appUpdater.js +++ b/server/services/appUpdater.js @@ -8,6 +8,7 @@ import { bufferedSpawnOrThrow } from '../lib/bufferedSpawn.js'; import { parseCommandArgs } from '../lib/commandSecurity.js'; import { isDetachedRunning, spawnDetached } from '../lib/detachedSpawn.js'; import { PORTOS_APP_ID } from '../lib/appIdentity.js'; +import { parseBuildCommand } from './appBuilder.js'; import { syncManagedAppFork } from './managedAppRepositories.js'; const CMD_TIMEOUT_MS = 5 * 60 * 1000; @@ -70,7 +71,10 @@ async function startDashboardHandoff(app) { * 2. install dependencies in each package directory (Bun apps use their * frozen lockfile; existing apps retain npm install) * 3. run setup with the same package manager when the script exists - * 4. Restart PM2 processes + * 4. rebuild the production UI when a build command or `scripts.build` exists + * (a pull that only restarts leaves `client/dist` stale and PortOS then + * reports "install out of sync") + * 5. Restart PM2 processes * * @param {object} app - The app object (must have repoPath, pm2ProcessNames, pm2Home) * @param {function} emit - Callback (step, status, message) for progress updates @@ -143,14 +147,28 @@ async function _doUpdate(app, emit, { syncFork }) { } const pkgPath = join(dir, 'package.json'); - if (existsSync(pkgPath)) { - const pkg = JSON.parse(await readFile(pkgPath, 'utf-8')); - if (pkg.scripts?.setup) { - emit('setup', 'running', 'Running setup...'); - await runCommand(packageManagerCommand, ['run', 'setup'], dir); - emit('setup', 'done', 'Setup complete'); - steps.push({ step: 'setup', success: true }); - } + const pkg = existsSync(pkgPath) ? JSON.parse(await readFile(pkgPath, 'utf-8')) : null; + if (pkg?.scripts?.setup) { + emit('setup', 'running', 'Running setup...'); + await runCommand(packageManagerCommand, ['run', 'setup'], dir); + emit('setup', 'done', 'Setup complete'); + steps.push({ step: 'setup', success: true }); + } + + const configuredBuild = typeof app.buildCommand === 'string' ? app.buildCommand.trim() : ''; + let build; + if (configuredBuild) { + const parsed = parseBuildCommand(configuredBuild); + if (!parsed.ok) throw new Error(parsed.message); + build = { cmd: parsed.cmd, args: parsed.args }; + } else if (pkg?.scripts?.build) { + build = { cmd: packageManagerCommand, args: ['run', 'build'] }; + } + if (build) { + emit('build', 'running', 'Building production UI...'); + await runCommand(build.cmd, build.args, dir); + emit('build', 'done', 'Production UI built'); + steps.push({ step: 'build', success: true }); } const processNames = app.pm2ProcessNames || []; diff --git a/server/services/appUpdater.test.js b/server/services/appUpdater.test.js index ba744df67e..2a7ba3f241 100644 --- a/server/services/appUpdater.test.js +++ b/server/services/appUpdater.test.js @@ -15,7 +15,10 @@ const mock = vi.hoisted(() => ({ vi.mock('./git.js', () => ({ pull: mock.pull })); vi.mock('./pm2.js', () => ({ restartApp: mock.restart })); -vi.mock('../lib/bufferedSpawn.js', () => ({ bufferedSpawnOrThrow: mock.spawn })); +vi.mock('../lib/bufferedSpawn.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, bufferedSpawnOrThrow: mock.spawn }; +}); vi.mock('../lib/detachedSpawn.js', () => ({ isDetachedRunning: mock.dashboardRunning, spawnDetached: mock.dashboardOpen, @@ -54,6 +57,7 @@ describe('managed app updates', () => { }); it('uses Bun and its frozen lockfile for Bun-managed apps', async () => { + await writeFile(join(repo, 'package.json'), JSON.stringify({ scripts: { setup: 'example-setup', build: 'vite build' } })); const emit = vi.fn(); const companionRepo = join(repo, '..', 'eidoverse-video'); const bunCommand = join(repo, 'tools with spaces', 'bun'); @@ -72,6 +76,7 @@ describe('managed app updates', () => { expect(mock.spawn).toHaveBeenCalledWith(bunCommand, ['install', '--frozen-lockfile'], expect.objectContaining({ cwd: repo })); expect(mock.spawn).toHaveBeenCalledWith(bunCommand, ['install', '--frozen-lockfile'], expect.objectContaining({ cwd: join(repo, 'client') })); expect(mock.spawn).toHaveBeenCalledWith(bunCommand, ['run', 'setup'], expect.objectContaining({ cwd: repo })); + expect(mock.spawn).toHaveBeenCalledWith(bunCommand, ['run', 'build'], expect.objectContaining({ cwd: repo })); expect(mock.spawn).not.toHaveBeenCalledWith('npm', expect.anything(), expect.anything()); expect(emit).toHaveBeenCalledWith('git-pull:companion-1', 'done', 'Already up to date'); expect(emit).toHaveBeenCalledWith('bun-install:root', 'done', 'root dependencies installed'); @@ -139,4 +144,68 @@ describe('managed app updates', () => { expect(mock.dashboardOpen).not.toHaveBeenCalled(); expect(mock.restart).toHaveBeenCalledWith('portos-server', undefined); }); + + it('rebuilds the production UI before restarting so a managed update does not leave a stale client bundle', async () => { + const emit = vi.fn(); + const managed = { + id: 'portos-default', + name: 'PortOS', + type: 'express', + repoPath: repo, + buildCommand: 'npm run build', + pm2ProcessNames: ['portos-server'], + }; + + const result = await updateApp(managed, emit); + + expect(result.success).toBe(true); + expect(result.steps.some((step) => step.step === 'build' && step.success)).toBe(true); + expect(mock.spawn).toHaveBeenCalledWith( + 'npm', + ['run', 'build'], + expect.objectContaining({ cwd: repo }), + ); + const buildCall = mock.spawn.mock.invocationCallOrder[ + mock.spawn.mock.calls.findIndex((call) => call[0] === 'npm' && call[1]?.[1] === 'build') + ]; + expect(buildCall).toBeLessThan(mock.restart.mock.invocationCallOrder[0]); + expect(emit).toHaveBeenCalledWith('build', 'done', 'Production UI built'); + }); + + it('rebuilds from package.json when no explicit build command is configured', async () => { + await writeFile(join(repo, 'package.json'), JSON.stringify({ scripts: { build: 'vite build' } })); + const emit = vi.fn(); + + await updateApp({ name: 'Example App', type: 'express', repoPath: repo, pm2ProcessNames: [] }, emit); + + expect(mock.spawn).toHaveBeenCalledWith( + 'npm', + ['run', 'build'], + expect.objectContaining({ cwd: repo }), + ); + expect(emit).toHaveBeenCalledWith('build', 'done', 'Production UI built'); + }); + + it('does not invent a production build when the app has none', async () => { + const emit = vi.fn(); + + await updateApp({ name: 'Example App', type: 'express', repoPath: repo, pm2ProcessNames: [] }, emit); + + expect(mock.spawn).not.toHaveBeenCalledWith('npm', ['run', 'build'], expect.anything()); + expect(emit).not.toHaveBeenCalledWith('build', expect.anything(), expect.anything()); + }); + + it('refuses a disallowed build command before restarting', async () => { + const emit = vi.fn(); + + await expect(updateApp({ + name: 'Example App', + type: 'express', + repoPath: repo, + buildCommand: 'rm -rf /', + pm2ProcessNames: ['example-app'], + }, emit)).rejects.toThrow(/not allowed/); + + expect(mock.restart).not.toHaveBeenCalled(); + }); });