diff --git a/AGENTS.md b/AGENTS.md index ebf5435..0bf4a52 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -125,6 +125,13 @@ registered in `src/commands/index.ts` and lazy-load their handlers from re-staging) — use it when changing RHDH Local configuration without touching plugin code. `update` re-exports, re-stages, and then restarts the RHDH service. +`start` and `update` both block on `waitForRhdhReady` (poll-based, 120s default +timeout) before returning, printing the RHDH URL once it responds. Both also +accept `--watch`, which hands off into `watchUpdate`: a chokidar watcher on +`src/`, `package.json`, and `tsconfig.json` (500ms debounce, serialized cycles) that repeats the +same export/stage/restart cycle as a one-shot `update` on every source change, +so a source edit doesn't require re-running the command by hand. + Key files: - `command.ts` — per-subcommand handlers (`start`, `update`, `restart`, `stop`, @@ -152,7 +159,11 @@ RHDH Local and will not appear in `git status` after a successful `start`. **Pre-flight check:** `start` and `update` call `validateProjectFiles()` before invoking `plugin export`. For backend plugins this checks that `dist-types/` exists, failing fast with a clear `yarn tsc` instruction rather than letting -`yarn build` fail deep in the export process. +`yarn build` fail deep in the export process. `update` and `restart` also call +`ensureRuntimeRunning()` first, which inspects Compose `ps` for the `rhdh` +service and fails fast with an actionable message ("RHDH Local is not running. +Run `rhdh-cli plugin dev start` first.") instead of a raw compose/container +subprocess error. **Symlink handling:** `stagePlugin` uses `fs.remove` + `fs.copy` with `dereference: false` so relative symlinks in `node_modules/.bin/` are preserved diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cd7baf..1106f3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Added + +- **`plugin dev`:** Add `--watch` to `rhdh-cli plugin dev start`, and a standalone `rhdh-cli plugin dev update --watch`, for continuous re-export/re-stage/restart on source changes ([RHIDP-16673](https://redhat.atlassian.net/browse/RHIDP-16673), [#222](https://github.com/redhat-developer/rhdh-cli/pull/222)). Watches `src/`, `package.json`, `tsconfig.json` with a 500ms debounce and serializes cycles so a change arriving mid-cycle queues exactly one follow-up; prints a refresh URL once RHDH responds. `plugin dev start` now also shows phased `[1/4]`–`[4/4]` progress through build/export, runtime start, plugin install, and readiness polling. `plugin dev update` and `plugin dev restart` fail fast with an actionable message when RHDH Local isn't running yet, instead of surfacing a raw compose/container error. + ## 2.1.1 - 2026-09-21 ### Added diff --git a/README.md b/README.md index da3f4a5..91fe1b1 100644 --- a/README.md +++ b/README.md @@ -88,12 +88,24 @@ Use `plugin dev` from a generated or existing dynamic plugin project to export i rhdh-cli plugin dev start --configure --rhdh-local-dir /path/to/rhdh-local ``` -`--configure` adds the CLI-managed plugin configuration include without replacing existing user configuration. Set `RHDH_LOCAL_DIR` to avoid repeating the path. After changing plugin source, refresh the staged plugin and RHDH service with: +`--configure` adds the CLI-managed plugin configuration include without replacing existing user configuration. Set `RHDH_LOCAL_DIR` to avoid repeating the path. `start` prints four labeled phases (`[1/4]` build/export, `[2/4]` start the runtime, `[3/4]` install plugins, `[4/4]` wait for readiness) and blocks until RHDH responds, printing the URL to open once it's ready. + +After changing plugin source, refresh the staged plugin and RHDH service with: ```bash rhdh-cli plugin dev update ``` +`update` and `restart` require the runtime to already be running — start it first with `plugin dev start`, or they fail fast with an actionable message instead of a raw compose/container error. Like `start`, `update` blocks until RHDH is reachable again (up to two minutes) and prints the URL on success. + +Pass `--watch` to `start` or `update` to keep the CLI running: it watches `src/`, `package.json`, and `tsconfig.json` and automatically re-exports, re-stages, and restarts the runtime on every change, so you don't have to re-run `update` by hand. + +```bash +rhdh-cli plugin dev start --watch +# or, once the runtime is already up: +rhdh-cli plugin dev update --watch +``` + Use `rhdh-cli plugin dev status` for the interpreted runtime state, `rhdh-cli plugin dev logs` for application logs, and `rhdh-cli plugin dev logs --installer` to diagnose installation failures. To restart the RHDH service after changing RHDH Local configuration (without re-deploying the plugin), use `rhdh-cli plugin dev restart`. Stop the runtime with `rhdh-cli plugin dev stop`; add `--clean` to remove containers and networks while retaining volumes, configuration, and plugin artifacts. The default container tool is `podman`; pass `--container-tool docker` if your environment uses Docker instead. The CLI manages a single plugin entry in `configs/dynamic-plugins/rhdh-cli.generated.local.yaml`. Each `start` or `update` run overwrites this file with the current plugin's package path, disabled flag, and pull policy. Extra `pluginConfig` for the plugin (such as app-config keys) belongs in `dynamic-plugins.override.yaml` under a `plugins:` entry for the same package, not in the generated file. diff --git a/package.json b/package.json index 769f2a2..c4038ec 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "@yarnpkg/lockfile": "^1.1.0", "@yarnpkg/parsers": "^3.0.0-rc.4", "chalk": "^4.0.0", + "chokidar": "^5.0.0", "codeowners": "^5.1.1", "commander": "^9.1.0", "eslint": "8.57.1", diff --git a/src/commands/dev/command.test.ts b/src/commands/dev/command.test.ts index 75d4a72..888ab5b 100644 --- a/src/commands/dev/command.test.ts +++ b/src/commands/dev/command.test.ts @@ -1,6 +1,8 @@ import fs from 'fs-extra'; import os from 'node:os'; import path from 'node:path'; +import EventEmitter from 'node:events'; +import chokidar from 'chokidar'; jest.mock('../../lib/paths', () => ({ paths: { @@ -40,6 +42,40 @@ jest.mock('../export-dynamic-plugin', () => ({ command: jest.fn(), })); +// --------------------------------------------------------------------------- +// Chokidar mock used by watchUpdate tests +// --------------------------------------------------------------------------- +class FakeWatcher extends EventEmitter { + close = jest.fn().mockResolvedValue(undefined); +} +let fakeWatcher: FakeWatcher; + +jest.mock('chokidar', () => ({ + __esModule: true, + default: { + watch: jest.fn(() => { + fakeWatcher = new FakeWatcher(); + return fakeWatcher; + }), + }, +})); + +// --------------------------------------------------------------------------- +// node:child_process spawn mock used by waitForContainerCleanup tests +// --------------------------------------------------------------------------- +class FakeChildProcess extends EventEmitter { + stdout = new EventEmitter(); + kill = jest.fn(); +} +let fakeChild: FakeChildProcess; + +jest.mock('node:child_process', () => ({ + spawn: jest.fn(() => { + fakeChild = new FakeChildProcess(); + return fakeChild; + }), +})); + import { composeArgs, composeStatusArgs, @@ -55,6 +91,15 @@ import { updateGeneratedConfig, stop, status, + start, + update, + restart, + watchUpdate, + waitForContainerEvent, + waitForContainerCleanup, + resolveRhdhUrl, + waitForRhdhReady, + isIgnoredWatchPath, } from './command'; import { Task } from '../../lib/tasks'; import { run, execFile } from '../../lib/run'; @@ -233,16 +278,19 @@ describe('plugin dev', () => { }); it('rejects a container tool that is not on PATH', async () => { - const taskMock = Task as jest.Mocked; - taskMock.forCommand.mockRejectedValueOnce(new Error('command not found')); + const execFileMock = execFile as jest.MockedFunction; + execFileMock.mockRejectedValueOnce(new Error('command not found')); await expect(validateContainerTool('docker')).rejects.toThrow( 'Unable to find docker on PATH', ); }); it('accepts a container tool that is on PATH', async () => { - const taskMock = Task as jest.Mocked; - taskMock.forCommand.mockResolvedValueOnce(undefined); + const execFileMock = execFile as jest.MockedFunction; + execFileMock.mockResolvedValueOnce({ + stdout: 'podman version 5.0.0', + stderr: '', + }); await expect(validateContainerTool('podman')).resolves.toBe('podman'); }); @@ -284,6 +332,12 @@ describe('plugin dev', () => { } mockRun.mockReset(); mockExecFile.mockReset(); + // validateContainerTool now uses execFile for the silent version check. + // Default to resolving so subcommand tests don't need to set it up themselves. + mockExecFile.mockResolvedValue({ + stdout: 'podman version 5.0.0', + stderr: '', + }); mockTask.forCommand.mockResolvedValue(undefined); mockTask.log.mockReset(); }); @@ -302,11 +356,91 @@ describe('plugin dev', () => { it('status rejects when the compose child process fails', async () => { const err = new ExitCodeError(1, 'podman compose ps'); - mockExecFile.mockRejectedValueOnce(err); + // First call: version check (passes); second call: compose ps (fails). + mockExecFile + .mockResolvedValueOnce({ stdout: 'podman version 5.0.0', stderr: '' }) + .mockRejectedValueOnce(err); await expect( status({ rhdhLocalDir: runtimeDir, containerTool: 'podman' }), ).rejects.toThrow(ExitCodeError); }); + + it('update rejects with an actionable message when RHDH Local is not running', async () => { + // First call: version check (passes); second call: compose ps reports + // no services at all (RHDH Local was never started). + mockExecFile + .mockResolvedValueOnce({ stdout: 'podman version 5.0.0', stderr: '' }) + .mockResolvedValueOnce({ stdout: '[]', stderr: '' }); + await expect( + update({ rhdhLocalDir: runtimeDir, containerTool: 'podman' }), + ).rejects.toThrow('RHDH Local is not running'); + }); + + it('update rejects when RHDH Local is stopped (not just absent)', async () => { + mockExecFile + .mockResolvedValueOnce({ stdout: 'podman version 5.0.0', stderr: '' }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { Service: 'rhdh', State: 'exited', ExitCode: 0 }, + ]), + stderr: '', + }); + await expect( + update({ rhdhLocalDir: runtimeDir, containerTool: 'podman' }), + ).rejects.toThrow('RHDH Local is not running'); + }); + + it('update --watch rejects immediately when RHDH Local is not running, without entering watch mode', async () => { + // Regression test: --watch must fail fast the same way the one-shot + // path does. Without an explicit ensureRuntimeRunning call before + // watchUpdate, this would instead resolve into watch mode and only + // surface the error on the first change-triggered cycle. + const mockChokidarWatch = chokidar.watch as jest.MockedFunction< + typeof chokidar.watch + >; + mockChokidarWatch.mockClear(); + mockExecFile + .mockResolvedValueOnce({ stdout: 'podman version 5.0.0', stderr: '' }) + .mockResolvedValueOnce({ stdout: '[]', stderr: '' }); + await expect( + update({ + rhdhLocalDir: runtimeDir, + containerTool: 'podman', + watch: true, + }), + ).rejects.toThrow('RHDH Local is not running'); + expect(mockChokidarWatch).not.toHaveBeenCalled(); + }); + + it('restart rejects with an actionable message when RHDH Local is not running', async () => { + mockExecFile + .mockResolvedValueOnce({ stdout: 'podman version 5.0.0', stderr: '' }) + .mockResolvedValueOnce({ stdout: '[]', stderr: '' }); + await expect( + restart({ rhdhLocalDir: runtimeDir, containerTool: 'podman' }), + ).rejects.toThrow('RHDH Local is not running'); + expect(mockRun).not.toHaveBeenCalled(); + }); + + it('restart stops and starts rhdh when RHDH Local is running', async () => { + // version check, then two compose-ps calls: the pre-flight check, then + // the final getRuntimeStatus call after stop-rhdh/start-rhdh. + mockExecFile + .mockResolvedValueOnce({ stdout: 'podman version 5.0.0', stderr: '' }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([{ Service: 'rhdh', State: 'running' }]), + stderr: '', + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([{ Service: 'rhdh', State: 'running' }]), + stderr: '', + }); + mockRun.mockResolvedValue(undefined); + await expect( + restart({ rhdhLocalDir: runtimeDir, containerTool: 'podman' }), + ).resolves.toBeUndefined(); + expect(mockRun).toHaveBeenCalledTimes(2); + }); }); it('reports the RHDH Local files missing from an incompatible directory', async () => { @@ -333,7 +467,7 @@ describe('plugin dev', () => { ); await expect( ensureGeneratedConfigIncluded(directory, false), - ).rejects.toThrow('rerun with --configure'); + ).rejects.toThrow('plugin dev start --configure'); await ensureGeneratedConfigIncluded(directory, true); await expect(fs.readFile(override, 'utf8')).resolves.toContain( 'rhdh-cli.generated.local.yaml', @@ -516,3 +650,1030 @@ describe('plugin dev', () => { } }); }); + +// --------------------------------------------------------------------------- +// isIgnoredWatchPath — chokidar v4+ dropped glob-string support for `ignored`; +// this is a regression test for that (a glob array previously matched nothing). +// --------------------------------------------------------------------------- + +describe('isIgnoredWatchPath', () => { + const root = path.join(path.sep, 'home', 'user', 'checkout', 'plugin'); + + it('ignores paths under output/dependency directories inside the plugin root', () => { + expect(isIgnoredWatchPath(path.join(root, 'dist', 'output.js'), root)).toBe( + true, + ); + expect( + isIgnoredWatchPath(path.join(root, 'dist-dynamic', 'package.json'), root), + ).toBe(true); + expect( + isIgnoredWatchPath(path.join(root, 'dist-types', 'index.d.ts'), root), + ).toBe(true); + expect( + isIgnoredWatchPath( + path.join(root, 'node_modules', 'pkg', 'index.js'), + root, + ), + ).toBe(true); + }); + + it('does not ignore ordinary watched paths', () => { + expect(isIgnoredWatchPath(path.join(root, 'src', 'index.ts'), root)).toBe( + false, + ); + expect(isIgnoredWatchPath(path.join(root, 'package.json'), root)).toBe( + false, + ); + }); + + it('does not ignore a path just because an ancestor outside the root is named dist or node_modules', () => { + // Regression test: chokidar always passes absolute paths. Checking every + // segment of the *full* absolute path (rather than only segments inside + // the plugin root) meant a checkout that merely lived under e.g. + // /home/user/dist/my-plugin or /home/user/node_modules-backup/my-plugin + // had every file ignored, while `watchUpdate` still printed "Watching...". + const weirdRoot = path.join(path.sep, 'home', 'user', 'dist', 'plugin'); + expect( + isIgnoredWatchPath(path.join(weirdRoot, 'src', 'index.ts'), weirdRoot), + ).toBe(false); + + const weirdRoot2 = path.join( + path.sep, + 'home', + 'user', + 'node_modules', + 'plugin', + ); + expect( + isIgnoredWatchPath(path.join(weirdRoot2, 'package.json'), weirdRoot2), + ).toBe(false); + }); + + it('does not ignore paths outside the root (defensive; chokidar should not call us with these)', () => { + expect( + isIgnoredWatchPath(path.join(path.sep, 'other', 'dist', 'file.js'), root), + ).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// watchUpdate — file-watching behaviour +// --------------------------------------------------------------------------- + +/** + * Scaffold a temp RHDH Local runtime dir (with the files + * `validateProjectFiles`/Compose checks require) and a temp plugin dir with a + * minimal frontend plugin `package.json`. Shared by the `watchUpdate` and + * `start` suites, which both drive a real (mocked-at-the-edges) `plugin dev` + * cycle against these directories. + */ +async function scaffoldPluginDevRuntime( + runtimePrefix: string, + pluginPrefix: string, + pluginPackageName: string, +): Promise<{ runtimeDir: string; pluginDir: string }> { + const runtimeDir = await fs.mkdtemp(path.join(os.tmpdir(), runtimePrefix)); + const pluginDir = await fs.mkdtemp(path.join(os.tmpdir(), pluginPrefix)); + (global as any).__pluginDevTestDir = pluginDir; + + for (const f of [ + 'compose.yaml', + 'compose-dynamic-plugins-root.yaml', + 'prepare-and-install-dynamic-plugins.sh', + 'wait-for-plugins-and-start.sh', + ]) { + await fs.writeFile(path.join(runtimeDir, f), ''); + } + + // Write the generated config include so ensureGeneratedConfigIncluded passes + const override = path.join( + runtimeDir, + 'configs/dynamic-plugins/dynamic-plugins.override.yaml', + ); + await fs.outputFile( + override, + `includes:\n - configs/dynamic-plugins/rhdh-cli.generated.local.yaml\n`, + ); + + // Write a minimal frontend plugin package.json so validateProjectFiles passes + // (frontend plugins do not require dist-types). + await fs.writeJson(path.join(pluginDir, 'package.json'), { + name: pluginPackageName, + version: '0.1.0', + backstage: { role: 'frontend-plugin' }, + }); + + return { runtimeDir, pluginDir }; +} + +describe('watchUpdate', () => { + let runtimeDir: string; + const mockRun = run as jest.MockedFunction; + const mockExecFile = execFile as jest.MockedFunction; + const mockTask = Task as jest.Mocked; + const mockExport = jest.requireMock('../export-dynamic-plugin') + .command as jest.MockedFunction<() => Promise>; + + let pluginDir: string; + + beforeEach(async () => { + ({ runtimeDir, pluginDir } = await scaffoldPluginDevRuntime( + 'plugin-dev-watch-runtime-', + 'plugin-dev-watch-plugin-', + '@internal/my-watch-plugin', + )); + + mockRun.mockReset(); + mockExecFile.mockReset(); + mockTask.forCommand.mockResolvedValue(undefined); + mockTask.log.mockReset(); + mockExport.mockReset(); + + // Default: export + compose succeed; compose ps reports rhdh running so + // ensureRuntimeRunning's pre-flight check passes. + mockExport.mockResolvedValue(undefined); + mockRun.mockResolvedValue(undefined); + mockExecFile.mockResolvedValue({ + stdout: JSON.stringify([{ Service: 'rhdh', State: 'running' }]), + stderr: '', + }); + }); + + afterEach(async () => { + delete (global as any).__pluginDevTestDir; + // Remove signal listeners added by watchUpdate to avoid accumulation + process.removeAllListeners('SIGINT'); + process.removeAllListeners('SIGTERM'); + await fs.remove(runtimeDir); + await fs.remove(pluginDir); + }); + + /** + * Wait until mockExport has been called at least `count` times by polling + * with setImmediate-based yields. We pass debounceMs=0 to watchUpdate so the + * setTimeout fires on the very next event loop tick; real timers mean fs-extra + * and other async operations resolve normally. + */ + async function waitForExportCalls(count: number, timeoutMs = 5000) { + const deadline = Date.now() + timeoutMs; + while (mockExport.mock.calls.length < count) { + if (Date.now() > deadline) { + throw new Error( + `Timed out waiting for mockExport to be called ${count} time(s) ` + + `(called ${mockExport.mock.calls.length} time(s))`, + ); + } + await new Promise(resolve => setImmediate(resolve)); + } + } + + /** + * Start watchUpdate and drive it into the settle-wait phase: cycle 1 is + * held open, a second change arrives while it's still in-flight (setting + * pendingChange), cycle 1 is then released, and this resolves once the + * settle-wait's two events subscriptions (rhdh + install-dynamic-plugins) + * have spawned. Shared by tests that need to interact with that window — + * injecting further changes, resolving it, or triggering shutdown. + */ + async function enterSettleWait(settleTimeoutMs: number): Promise<{ + watchPromise: Promise; + settleChildren: FakeChildProcess[]; + spawnMock: jest.Mock; + }> { + const { spawn: spawnMock } = jest.requireMock('node:child_process') as { + spawn: jest.Mock; + }; + const settleChildren: FakeChildProcess[] = []; + spawnMock.mockImplementation(() => { + const child = new FakeChildProcess(); + settleChildren.push(child); + return child; + }); + + // Hold cycle 1 open so we can reliably inject a second change while + // `running` is still true. + let releaseCycle1: () => void = () => {}; + mockExport.mockImplementationOnce( + () => new Promise(resolve => (releaseCycle1 = resolve)), + ); + + const watchPromise = watchUpdate('podman', runtimeDir, 0, settleTimeoutMs); + + fakeWatcher.emit('all', 'change', 'src/a.ts'); + await waitForExportCalls(1); // cycle 1 started; still in-flight + + fakeWatcher.emit('all', 'change', 'src/b.ts'); + // Let the second change's own (0ms) debounce timer fire and observe + // running === true, setting pendingChange instead of starting a second + // drainCycles(). + await new Promise(resolve => setTimeout(resolve, 10)); + + releaseCycle1(); // cycle 1 finishes; keepGoing becomes true + + const spawnDeadline = Date.now() + 5000; + while (settleChildren.length < 2) { + if (Date.now() > spawnDeadline) { + throw new Error('Timed out waiting for settle-wait children to spawn'); + } + await new Promise(resolve => setImmediate(resolve)); + } + + return { watchPromise, settleChildren, spawnMock }; + } + + /** Emit the settle-wait's expected 'cleanup' event to each captured child. */ + function resolveSettleWait(settleChildren: FakeChildProcess[]) { + for (const child of settleChildren) { + const rhdhEvent = JSON.stringify({ + Status: 'cleanup', + Attributes: { 'com.docker.compose.service': 'rhdh' }, + }); + child.stdout.emit('data', Buffer.from(`${rhdhEvent}\n`)); + const installerEvent = JSON.stringify({ + Status: 'cleanup', + Attributes: { + 'com.docker.compose.service': 'install-dynamic-plugins', + }, + }); + child.stdout.emit('data', Buffer.from(`${installerEvent}\n`)); + } + } + + /** Restore spawn's default mock (tracking the shared `fakeChild`). */ + function restoreDefaultSpawnMock(spawnMock: jest.Mock) { + spawnMock.mockImplementation(() => { + fakeChild = new FakeChildProcess(); + return fakeChild; + }); + } + + it('runs an update cycle when a file change event fires', async () => { + // debounceMs=0 so the timer fires on the next event-loop tick. + const watchPromise = watchUpdate('podman', runtimeDir, 0, 0); + + fakeWatcher.emit('all', 'change', 'src/index.ts'); + await waitForExportCalls(1); + + expect(mockExport).toHaveBeenCalledTimes(1); + + fakeWatcher.emit('error', new Error('done')); + await expect(watchPromise).rejects.toThrow('done'); + }); + + it('watches tsconfig.json in addition to src/ and package.json', async () => { + // Regression test: RHIDP-16673's acceptance criteria require watching + // "relevant build configuration files" alongside src/ and package.json. + // tsconfig.json is the only one scaffolded projects actually have + // (plugin new's adaptStandaloneProject). + const mockChokidarWatch = chokidar.watch as jest.MockedFunction< + typeof chokidar.watch + >; + mockChokidarWatch.mockClear(); + + const watchPromise = watchUpdate('podman', runtimeDir, 0, 0); + await new Promise(resolve => setImmediate(resolve)); + + expect(mockChokidarWatch).toHaveBeenCalledWith( + expect.arrayContaining([expect.stringContaining('tsconfig.json')]), + expect.anything(), + ); + + fakeWatcher.emit('error', new Error('done')); + await expect(watchPromise).rejects.toThrow('done'); + }); + + it('debounces rapid consecutive file events into a single cycle', async () => { + // Use debounceMs=20 so rapid events within that window coalesce, but the + // cycle still completes quickly in real-timer mode. + const watchPromise = watchUpdate('podman', runtimeDir, 20, 0); + + // Three events fired rapidly — only the first one should schedule a timer + // (the guard `if (debounceTimer !== undefined) return` drops the rest). + fakeWatcher.emit('all', 'change', 'src/a.ts'); + fakeWatcher.emit('all', 'change', 'src/b.ts'); + fakeWatcher.emit('all', 'change', 'src/c.ts'); + await waitForExportCalls(1); + + expect(mockExport).toHaveBeenCalledTimes(1); + + fakeWatcher.emit('error', new Error('done')); + await expect(watchPromise).rejects.toThrow('done'); + }); + + it('holds running through the settle wait, so a change during it does not start a concurrent cycle', async () => { + // Regression test: `running` was previously cleared in a `finally` block + // right after the cycle itself, before the optional settle-wait ran. A + // change arriving during that wait saw `running === false` and called + // drainCycles() again — a second, concurrent loop rather than a + // coalesced follow-up. enterSettleWait() already proves the settle-wait + // subscribes to the events stream *before* runUpdateCycle's own compose + // stop/start calls run (not after), by capturing spawned children and + // confirming both exist before cycle 2 begins. + const { watchPromise, settleChildren, spawnMock } = + await enterSettleWait(5000); + + // While the settle-wait is unresolved, cycle 2 must not have started yet + // — proving `running` stayed true and the pending change was coalesced + // into this same drainCycles() loop rather than a concurrent one. + await new Promise(resolve => setImmediate(resolve)); + expect(mockExport).toHaveBeenCalledTimes(1); + + // A third change arriving *during* the settle-wait is the critical case: + // with the bug (running cleared before the wait), this would see + // running === false and call drainCycles() again — a second, concurrent + // cycle starting immediately, incrementing mockExport to 2 before the + // settle-wait ever resolves. With the fix, it's coalesced into the same + // pendingChange the already-running loop will pick up once the wait ends. + fakeWatcher.emit('all', 'change', 'src/c.ts'); + await new Promise(resolve => setTimeout(resolve, 10)); + expect(mockExport).toHaveBeenCalledTimes(1); + + resolveSettleWait(settleChildren); + + // Cycle 2 now runs, absorbing the coalesced pending change. + await waitForExportCalls(2); + expect(mockExport).toHaveBeenCalledTimes(2); + + fakeWatcher.emit('error', new Error('done')); + await expect(watchPromise).rejects.toThrow('done'); + + restoreDefaultSpawnMock(spawnMock); + }); + + it('continues watching after a failed update cycle', async () => { + const watchPromise = watchUpdate('podman', runtimeDir, 0, 0); + + mockExport.mockRejectedValueOnce(new Error('build exploded')); + + fakeWatcher.emit('all', 'change', 'src/fail.ts'); + await waitForExportCalls(1); + + expect(mockExport).toHaveBeenCalledTimes(1); + expect(mockTask.log).toHaveBeenCalledWith( + expect.stringContaining('build exploded'), + ); + + // A second event after the failed cycle should still trigger a new cycle. + mockExport.mockResolvedValueOnce(undefined); + fakeWatcher.emit('all', 'change', 'src/fixed.ts'); + await waitForExportCalls(2); + + expect(mockExport).toHaveBeenCalledTimes(2); + + fakeWatcher.emit('error', new Error('done')); + await expect(watchPromise).rejects.toThrow('done'); + }); + + async function waitForTaskLogContaining(substring: string, timeoutMs = 5000) { + const deadline = Date.now() + timeoutMs; + while ( + !mockTask.log.mock.calls.some(call => String(call[0]).includes(substring)) + ) { + if (Date.now() > deadline) { + throw new Error( + `Timed out waiting for a Task.log call containing "${substring}"`, + ); + } + await new Promise(resolve => setImmediate(resolve)); + } + } + + it('fails a cycle cleanly and keeps watching when RHDH Local is not running', async () => { + const watchPromise = watchUpdate('podman', runtimeDir, 0, 0); + + // Simulate RHDH Local not running for the first triggered cycle — + // ensureRuntimeRunning should reject before exportCommand is ever called. + mockExecFile.mockResolvedValueOnce({ stdout: '[]', stderr: '' }); + + fakeWatcher.emit('all', 'change', 'src/index.ts'); + await waitForTaskLogContaining('RHDH Local is not running'); + expect(mockExport).not.toHaveBeenCalled(); + + // A subsequent change, with RHDH running again (the default mock), should + // succeed and the watcher should still be alive to pick it up. + fakeWatcher.emit('all', 'change', 'src/fixed.ts'); + await waitForExportCalls(1); + expect(mockExport).toHaveBeenCalledTimes(1); + + fakeWatcher.emit('error', new Error('done')); + await expect(watchPromise).rejects.toThrow('done'); + }); + + it('closes the watcher on SIGINT', async () => { + watchUpdate('podman', runtimeDir, 0, 0); + + const exitSpy = jest + .spyOn(process, 'exit') + .mockImplementation((() => {}) as () => never); + + process.emit('SIGINT'); + + // Allow the shutdown async chain to run. + await new Promise(resolve => setImmediate(resolve)); + await Promise.resolve(); + + expect(fakeWatcher.close).toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(0); + + exitSpy.mockRestore(); + }); + + it('closes the watcher on SIGTERM', async () => { + // Regression coverage alongside the SIGINT test above: a terminal Ctrl+C + // signals the whole foreground process group for free, but a direct + // SIGTERM to just this process's PID does not reach any spawned + // children — shutdown() must still run the same way on either signal. + watchUpdate('podman', runtimeDir, 0, 0); + + const exitSpy = jest + .spyOn(process, 'exit') + .mockImplementation((() => {}) as () => never); + + process.emit('SIGTERM'); + + await new Promise(resolve => setImmediate(resolve)); + await Promise.resolve(); + + expect(fakeWatcher.close).toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(0); + + exitSpy.mockRestore(); + }); + + it('kills an in-flight settle-wait events subscription on shutdown', async () => { + // Regression test: spawn() puts children in the same process group as + // the parent by default, so SIGINT (terminal-wide) takes them along for + // free, but SIGTERM to just this PID does not — and process.exit() + // prevents waitForContainerEvent's own JS-side timeout from ever running + // to kill them itself. shutdown() must kill any tracked child directly. + const { watchPromise, settleChildren, spawnMock } = + await enterSettleWait(5000); + + const exitSpy = jest + .spyOn(process, 'exit') + .mockImplementation((() => {}) as () => never); + + process.emit('SIGTERM'); + await new Promise(resolve => setImmediate(resolve)); + await Promise.resolve(); + + for (const child of settleChildren) { + expect(child.kill).toHaveBeenCalled(); + } + + exitSpy.mockRestore(); + void watchPromise; + + restoreDefaultSpawnMock(spawnMock); + }); + + it('update --watch deploys the current tree immediately, before entering watch mode', async () => { + // Regression test: unlike `start --watch` (which runs its full 4-phase + // cycle before conditionally entering watchUpdate), `update --watch` + // previously skipped straight into watchUpdate() with no initial + // deploy — whatever was already on disk sat undeployed until the first + // change-triggered cycle. + // stagePlugin requires dist-dynamic to already exist (export is mocked + // and doesn't create it itself). + await fs.ensureDir(path.join(pluginDir, 'dist-dynamic')); + await fs.writeJson(path.join(pluginDir, 'dist-dynamic', 'package.json'), { + name: '@internal/my-watch-plugin', + version: '0.1.0', + }); + + const mockFetch = jest + .fn() + .mockResolvedValue(new Response(null, { status: 200 }) as Response); + (global as any).fetch = mockFetch; + + const updatePromise = update({ + rhdhLocalDir: runtimeDir, + containerTool: 'podman', + watch: true, + }); + + // The initial cycle runs immediately, without waiting for any watcher + // event. + await waitForExportCalls(1); + await waitForTaskLogContaining('Refresh your browser at'); + + // A subsequent file change triggers a second cycle via watchUpdate. + fakeWatcher.emit('all', 'change', 'src/index.ts'); + await waitForExportCalls(2); + expect(mockExport).toHaveBeenCalledTimes(2); + + fakeWatcher.emit('error', new Error('done')); + await expect(updatePromise).rejects.toThrow('done'); + + (global as any).fetch = undefined; + }); +}); + +// --------------------------------------------------------------------------- +// start — --watch wiring +// --------------------------------------------------------------------------- + +describe('start', () => { + let runtimeDir: string; + let pluginDir: string; + const mockRun = run as jest.MockedFunction; + const mockExecFile = execFile as jest.MockedFunction; + const mockTask = Task as jest.Mocked; + const mockExport = jest.requireMock('../export-dynamic-plugin') + .command as jest.MockedFunction<() => Promise>; + const mockFetch = jest.fn< + ReturnType, + Parameters + >(); + const mockChokidarWatch = chokidar.watch as jest.MockedFunction< + typeof chokidar.watch + >; + + beforeEach(async () => { + ({ runtimeDir, pluginDir } = await scaffoldPluginDevRuntime( + 'plugin-dev-start-runtime-', + 'plugin-dev-start-plugin-', + '@internal/my-start-plugin', + )); + // stagePlugin requires dist-dynamic to already exist (export is mocked). + await fs.ensureDir(path.join(pluginDir, 'dist-dynamic')); + await fs.writeJson(path.join(pluginDir, 'dist-dynamic', 'package.json'), { + name: '@internal/my-start-plugin', + version: '0.1.0', + }); + + mockRun.mockReset(); + mockExecFile.mockReset(); + mockTask.forCommand.mockResolvedValue(undefined); + mockTask.log.mockReset(); + mockExport.mockReset(); + mockChokidarWatch.mockClear(); + (jest.requireMock('node:child_process').spawn as jest.Mock).mockClear(); + + mockExport.mockResolvedValue(undefined); + mockRun.mockResolvedValue(undefined); + mockExecFile.mockResolvedValue({ + stdout: 'podman version 5.0.0', + stderr: '', + }); + + mockFetch.mockReset(); + (global as any).fetch = mockFetch; + mockFetch.mockResolvedValue( + new Response(null, { status: 200 }) as Response, + ); + jest.spyOn(process.stdout, 'write').mockImplementation(() => true); + }); + + afterEach(async () => { + delete (global as any).__pluginDevTestDir; + (process.stdout.write as jest.Mock).mockRestore(); + (global as any).fetch = undefined; + process.removeAllListeners('SIGINT'); + process.removeAllListeners('SIGTERM'); + await fs.remove(runtimeDir); + await fs.remove(pluginDir); + }); + + const { spawn: spawnMock } = jest.requireMock('node:child_process') as { + spawn: jest.Mock; + }; + + function emitInstallerDied() { + const event = JSON.stringify({ + Action: 'died', + Attributes: { 'com.docker.compose.service': 'install-dynamic-plugins' }, + }); + fakeChild.stdout.emit('data', Buffer.from(`${event}\n`)); + } + + async function waitForCondition( + check: () => boolean, + description: string, + timeoutMs = 5000, + ) { + const deadline = Date.now() + timeoutMs; + while (!check()) { + if (Date.now() > deadline) { + throw new Error(`Timed out waiting for: ${description}`); + } + await new Promise(resolve => setImmediate(resolve)); + } + } + + it('enters watch mode after a successful start when --watch is passed', async () => { + const startPromise = start({ + rhdhLocalDir: runtimeDir, + containerTool: 'podman', + watch: true, + }); + + // Phases 1-2 (export/stage/compose-start) involve real fs I/O, so poll + // rather than assume a fixed number of ticks — wait until phase 3 + // actually spawns the container-events subscription, then satisfy it. + await waitForCondition( + () => spawnMock.mock.calls.length > 0, + 'waitForContainerEvent to spawn the events subscription', + ); + emitInstallerDied(); + + // Phase 4's readiness poll resolves on the first mocked fetch; once + // start() proceeds into watchUpdate, chokidar.watch is called synchronously. + await waitForCondition( + () => mockChokidarWatch.mock.calls.length > 0, + 'watchUpdate to call chokidar.watch', + ); + + expect(mockTask.log).toHaveBeenCalledWith( + expect.stringContaining('RHDH is ready at'), + ); + + fakeWatcher.emit('error', new Error('done')); + await expect(startPromise).rejects.toThrow('done'); + }); + + it('does not enter watch mode when --watch is not passed', async () => { + const startPromise = start({ + rhdhLocalDir: runtimeDir, + containerTool: 'podman', + }); + + await waitForCondition( + () => spawnMock.mock.calls.length > 0, + 'waitForContainerEvent to spawn the events subscription', + ); + emitInstallerDied(); + + await expect(startPromise).resolves.toBeUndefined(); + expect(mockChokidarWatch).not.toHaveBeenCalled(); + }); + + it('skips waiting for the installer event when it already exited (re-entrant start)', async () => { + // version check, then compose ps reporting the installer already exited + // from a previous `start` — compose up -d won't restart it, so its + // `died` event will never fire again for a fresh event-stream subscriber. + mockExecFile + .mockResolvedValueOnce({ stdout: 'podman version 5.0.0', stderr: '' }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { Service: 'install-dynamic-plugins', State: 'exited', ExitCode: 0 }, + ]), + stderr: '', + }); + + const startPromise = start({ + rhdhLocalDir: runtimeDir, + containerTool: 'podman', + }); + + await expect(startPromise).resolves.toBeUndefined(); + // Never needed to subscribe to the events stream at all. + expect(spawnMock).not.toHaveBeenCalled(); + }); + + it("waits for docker's 'die' event, not podman's 'died', when --container-tool=docker", async () => { + // Regression test: Docker's terminal container-death event is 'die', not + // 'died'. Subscribing to phase 3's events stream with the wrong filter + // means it never matches anything and stalls for the full + // waitForContainerEvent timeout on every Docker-backed `start`. + const startPromise = start({ + rhdhLocalDir: runtimeDir, + containerTool: 'docker', + }); + + await waitForCondition( + () => spawnMock.mock.calls.length > 0, + 'waitForContainerEvent to spawn the events subscription', + ); + const [, spawnArgs] = spawnMock.mock.calls[0] as [string, string[]]; + expect(spawnArgs).toContain('event=die'); + expect(spawnArgs).not.toContain('event=died'); + + // Real Docker `events --format json` shape: the compose-service label + // lives under Actor.Attributes, not a top-level Attributes field (that's + // Podman's shape — using it here would silently pass this test without + // actually exercising Docker's real event parsing). + const event = JSON.stringify({ + Action: 'die', + Actor: { + Attributes: { 'com.docker.compose.service': 'install-dynamic-plugins' }, + }, + }); + fakeChild.stdout.emit('data', Buffer.from(`${event}\n`)); + + await expect(startPromise).resolves.toBeUndefined(); + }); +}); + +// --------------------------------------------------------------------------- +// waitForContainerEvent — single-service event subscription +// --------------------------------------------------------------------------- + +describe('waitForContainerEvent', () => { + function emitEvent(svc: string, action: string) { + const event = JSON.stringify({ + Action: action, + Attributes: { 'com.docker.compose.service': svc }, + }); + fakeChild.stdout.emit('data', Buffer.from(`${event}\n`)); + } + + it('resolves when the target service emits the target action', async () => { + const p = waitForContainerEvent('podman', 'rhdh', 'died', 5000); + await new Promise(resolve => setImmediate(resolve)); + emitEvent('rhdh', 'died'); + await new Promise(resolve => setImmediate(resolve)); + await expect(p).resolves.toBeUndefined(); + expect(fakeChild.kill).toHaveBeenCalled(); + }); + + it('ignores events for other services', async () => { + const p = waitForContainerEvent('podman', 'rhdh', 'died', 5000); + await new Promise(resolve => setImmediate(resolve)); + // wrong service — should not resolve + emitEvent('install-dynamic-plugins', 'died'); + await new Promise(resolve => setImmediate(resolve)); + // correct service + emitEvent('rhdh', 'died'); + await new Promise(resolve => setImmediate(resolve)); + await expect(p).resolves.toBeUndefined(); + }); + + it('resolves on timeout when the event does not arrive', async () => { + jest.useFakeTimers(); + const p = waitForContainerEvent('podman', 'rhdh', 'died', 1000); + await Promise.resolve(); + jest.advanceTimersByTime(1100); + for (let i = 0; i < 5; i++) await Promise.resolve(); + await expect(p).resolves.toBeUndefined(); + expect(fakeChild.kill).toHaveBeenCalled(); + jest.useRealTimers(); + }); + + it('passes the correct event filter to spawn', async () => { + const { spawn: spawnMock } = jest.requireMock('node:child_process') as { + spawn: jest.Mock; + }; + spawnMock.mockClear(); + + waitForContainerEvent('docker', 'install-dynamic-plugins', 'die', 5000); + await new Promise(resolve => setImmediate(resolve)); + + expect(spawnMock).toHaveBeenCalledWith( + 'docker', + expect.arrayContaining([ + 'event=die', + 'label=com.docker.compose.service=install-dynamic-plugins', + ]), + expect.anything(), + ); + }); + + it('omits --stream for docker, which has no such flag and always streams', async () => { + // Regression test: docker's `events` command has no --stream flag and + // rejects it, exiting immediately (child.on('close') would then resolve + // this promise before any event is ever seen). Podman's `events` + // supports (and per its own docs, expects) an explicit --stream. + const { spawn: spawnMock } = jest.requireMock('node:child_process') as { + spawn: jest.Mock; + }; + spawnMock.mockClear(); + + waitForContainerEvent('docker', 'rhdh', 'die', 5000); + await new Promise(resolve => setImmediate(resolve)); + const [, dockerArgs] = spawnMock.mock.calls[0] as [string, string[]]; + expect(dockerArgs).not.toContain('--stream'); + + spawnMock.mockClear(); + waitForContainerEvent('podman', 'rhdh', 'died', 5000); + await new Promise(resolve => setImmediate(resolve)); + const [, podmanArgs] = spawnMock.mock.calls[0] as [string, string[]]; + expect(podmanArgs).toContain('--stream'); + }); + + it("matches Docker's Actor.Attributes event shape, not just Podman's top-level Attributes", async () => { + // Regression test: real `docker events --format json` output nests the + // compose-service label under Actor.Attributes, not a top-level + // Attributes field. Podman puts it top-level. Reading only the top-level + // field means svc is always '' for real Docker events, so the filter + // never matches and this always burns the full timeout on Docker. + const p = waitForContainerEvent('docker', 'rhdh', 'die', 5000); + await new Promise(resolve => setImmediate(resolve)); + const event = JSON.stringify({ + Action: 'die', + Actor: { Attributes: { 'com.docker.compose.service': 'rhdh' } }, + }); + fakeChild.stdout.emit('data', Buffer.from(`${event}\n`)); + await new Promise(resolve => setImmediate(resolve)); + await expect(p).resolves.toBeUndefined(); + expect(fakeChild.kill).toHaveBeenCalled(); + }); + + it('still matches events using the top-level Attributes shape (Podman)', async () => { + const p = waitForContainerEvent('podman', 'rhdh', 'died', 5000); + await new Promise(resolve => setImmediate(resolve)); + emitEvent('rhdh', 'died'); + await new Promise(resolve => setImmediate(resolve)); + await expect(p).resolves.toBeUndefined(); + }); + + it('matches events using the lowercase status field as an action fallback', async () => { + const p = waitForContainerEvent('docker', 'rhdh', 'die', 5000); + await new Promise(resolve => setImmediate(resolve)); + fakeChild.stdout.emit( + 'data', + Buffer.from( + `${JSON.stringify({ + status: 'die', + Actor: { Attributes: { 'com.docker.compose.service': 'rhdh' } }, + })}\n`, + ), + ); + await new Promise(resolve => setImmediate(resolve)); + await expect(p).resolves.toBeUndefined(); + expect(fakeChild.kill).toHaveBeenCalled(); + }); + + it('logs a warning and still resolves when the events command fails to spawn', async () => { + const mockTask = Task as jest.Mocked; + mockTask.log.mockClear(); + const p = waitForContainerEvent('podman', 'rhdh', 'died', 5000); + await new Promise(resolve => setImmediate(resolve)); + fakeChild.emit('error', new Error('spawn podman ENOENT')); + await expect(p).resolves.toBeUndefined(); + expect(mockTask.log).toHaveBeenCalledWith( + expect.stringContaining('spawn podman ENOENT'), + ); + }); + + it('logs a warning when the events command exits unexpectedly on its own', async () => { + const mockTask = Task as jest.Mocked; + mockTask.log.mockClear(); + const p = waitForContainerEvent('podman', 'rhdh', 'died', 5000); + await new Promise(resolve => setImmediate(resolve)); + fakeChild.emit('close', 1, null); + await expect(p).resolves.toBeUndefined(); + expect(mockTask.log).toHaveBeenCalledWith( + expect.stringContaining('exited unexpectedly (code 1)'), + ); + }); + + it('does not log a warning when close follows our own kill() after a match', async () => { + const mockTask = Task as jest.Mocked; + mockTask.log.mockClear(); + const p = waitForContainerEvent('podman', 'rhdh', 'died', 5000); + await new Promise(resolve => setImmediate(resolve)); + emitEvent('rhdh', 'died'); + await new Promise(resolve => setImmediate(resolve)); + await p; + // Simulate the 'close' event that naturally follows our own kill() — + // code is null (terminated by signal), not a non-zero exit code. + fakeChild.emit('close', null, 'SIGTERM'); + expect(mockTask.log).not.toHaveBeenCalledWith( + expect.stringContaining('exited unexpectedly'), + ); + }); +}); + +// --------------------------------------------------------------------------- +// waitForContainerCleanup — dual-service settle (delegates to waitForContainerEvent) +// --------------------------------------------------------------------------- + +describe('waitForContainerCleanup', () => { + it('uses cleanup for podman and die for docker', async () => { + const { spawn: spawnMock } = jest.requireMock('node:child_process') as { + spawn: jest.Mock; + }; + spawnMock.mockClear(); + + // waitForContainerCleanup spawns two parallel waitForContainerEvent calls. + // Capture each FakeChildProcess instance as spawn is called. + const children: FakeChildProcess[] = []; + spawnMock.mockImplementation(() => { + const child = new FakeChildProcess(); + children.push(child); + return child; + }); + + const p = waitForContainerCleanup('podman', 5000); + // Allow both spawns to register their stdout listeners. + await new Promise(resolve => setImmediate(resolve)); + await new Promise(resolve => setImmediate(resolve)); + + const calls = spawnMock.mock.calls as Array<[string, string[]]>; + expect(calls.every(([tool]) => tool === 'podman')).toBe(true); + expect(calls.some(([, args]) => args.includes('event=cleanup'))).toBe(true); + expect(calls.some(([, args]) => args.includes('event=die'))).toBe(false); + + // Emit the cleanup event to each respective child. + function emitTo(child: FakeChildProcess, svc: string, action = 'cleanup') { + const event = JSON.stringify({ + Action: action, + Attributes: { 'com.docker.compose.service': svc }, + }); + child.stdout.emit('data', Buffer.from(`${event}\n`)); + } + // children[0] watches rhdh, children[1] watches install-dynamic-plugins + // (or vice-versa depending on Promise.all order — emit to both). + for (const child of children) { + emitTo(child, 'rhdh'); + emitTo(child, 'install-dynamic-plugins'); + } + await new Promise(resolve => setImmediate(resolve)); + await expect(p).resolves.toBeUndefined(); + + // Restore the default mock implementation for subsequent tests. + spawnMock.mockImplementation(() => { + fakeChild = new FakeChildProcess(); + return fakeChild; + }); + }); +}); + +// --------------------------------------------------------------------------- +// resolveRhdhUrl — env file parsing +// --------------------------------------------------------------------------- + +describe('resolveRhdhUrl', () => { + let dir: string; + + beforeEach(async () => { + dir = await fs.mkdtemp(path.join(os.tmpdir(), 'plugin-dev-url-')); + }); + afterEach(() => fs.remove(dir)); + + const writeEnvFile = (name: string, content: string) => + fs.writeFile(path.join(dir, name), content); + + it('returns the fallback URL when no env files exist', async () => { + await expect(resolveRhdhUrl(dir)).resolves.toBe('http://localhost:7007'); + }); + + it('reads BASE_URL from default.env', async () => { + await writeEnvFile('default.env', 'BASE_URL=http://localhost:9999\n'); + await expect(resolveRhdhUrl(dir)).resolves.toBe('http://localhost:9999'); + }); + + it('.env overrides default.env', async () => { + await writeEnvFile('default.env', 'BASE_URL=http://localhost:9999\n'); + await writeEnvFile('.env', '# comment\nBASE_URL=http://my-host:7007\n'); + await expect(resolveRhdhUrl(dir)).resolves.toBe('http://my-host:7007'); + }); + + it('ignores commented-out BASE_URL lines', async () => { + await writeEnvFile('default.env', 'BASE_URL=http://localhost:7007\n'); + await writeEnvFile('.env', '# BASE_URL=http://other:1234\n'); + await expect(resolveRhdhUrl(dir)).resolves.toBe('http://localhost:7007'); + }); +}); + +// --------------------------------------------------------------------------- +// waitForRhdhReady — HTTP readiness polling +// --------------------------------------------------------------------------- + +describe('waitForRhdhReady', () => { + const mockFetch = jest.fn< + ReturnType, + Parameters + >(); + + beforeEach(() => { + mockFetch.mockReset(); + // Replace global fetch with the mock for the duration of each test. + (global as any).fetch = mockFetch; + // Suppress the dot output during tests. + jest.spyOn(process.stdout, 'write').mockImplementation(() => true); + }); + + afterEach(() => { + (process.stdout.write as jest.Mock).mockRestore(); + (global as any).fetch = undefined; + }); + + it('resolves with the URL when the first poll returns 200', async () => { + mockFetch.mockResolvedValueOnce( + new Response(null, { status: 200 }) as Response, + ); + await expect( + waitForRhdhReady('http://localhost:7007', 5000, 0), + ).resolves.toBe('http://localhost:7007'); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('retries on connection failure and resolves when it succeeds', async () => { + mockFetch + .mockRejectedValueOnce(new Error('ECONNREFUSED')) + .mockResolvedValueOnce(new Response(null, { status: 200 }) as Response); + await expect( + waitForRhdhReady('http://localhost:7007', 5000, 0), + ).resolves.toBe('http://localhost:7007'); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('throws after timeout when never ready', async () => { + mockFetch.mockRejectedValue(new Error('ECONNREFUSED')); + await expect( + waitForRhdhReady('http://localhost:7007', 10, 0), + ).rejects.toThrow('did not become ready'); + }); +}); diff --git a/src/commands/dev/command.ts b/src/commands/dev/command.ts index 149d70b..eb24973 100644 --- a/src/commands/dev/command.ts +++ b/src/commands/dev/command.ts @@ -18,12 +18,77 @@ import { OptionValues } from 'commander'; import fs from 'fs-extra'; import YAML from 'yaml'; import path from 'node:path'; +import { spawn, ChildProcess } from 'node:child_process'; +import chokidar from 'chokidar'; import { execFile, run } from '../../lib/run'; import { Task } from '../../lib/tasks'; import { paths } from '../../lib/paths'; import { command as exportCommand } from '../export-dynamic-plugin'; +/** Default readiness poll timeout for `waitForRhdhReady`. */ +const READY_TIMEOUT_MS = 120_000; +/** Interval between readiness poll attempts. */ +const READY_POLL_INTERVAL_MS = 2_000; + +/** + * Read `BASE_URL` from the rhdh-local env files (`default.env`, then `.env`). + * Falls back to `http://localhost:7007` if neither file defines the key. + * The `.env` file is optional and may override `default.env` values. + */ +export async function resolveRhdhUrl(runtimeDir: string): Promise { + const fallback = 'http://localhost:7007'; + let url = fallback; + for (const name of ['default.env', '.env']) { + const file = path.join(runtimeDir, name); + if (!(await fs.pathExists(file))) continue; + const content = await fs.readFile(file, 'utf8'); + for (const line of content.split('\n')) { + const eq = line.indexOf('='); + if (eq === -1 || line.slice(0, eq).trim() !== 'BASE_URL') continue; + const value = line + .slice(eq + 1) + .trim() + .replace(/^["']|["']$/g, ''); + if (value) url = value; + } + } + return url; +} + +/** + * Poll `url` until it returns HTTP 2xx, then return the URL. + * Prints a dot to stdout every `pollIntervalMs` while waiting. + * Throws if `timeoutMs` elapses without a successful response. + */ +export async function waitForRhdhReady( + url: string, + timeoutMs = READY_TIMEOUT_MS, + pollIntervalMs = READY_POLL_INTERVAL_MS, +): Promise { + const deadline = Date.now() + timeoutMs; + process.stdout.write(`Waiting for ${url} `); + while (Date.now() < deadline) { + try { + const res = await fetch(url, { signal: AbortSignal.timeout(5_000) }); + // 3xx redirects are treated as ready too, not just res.ok's [200, 300). + if (res.status >= 200 && res.status < 400) { + process.stdout.write(' ready.\n'); + return url; + } + } catch { + // Connection refused, timeout, etc. — keep polling. + } + process.stdout.write('.'); + await new Promise(resolve => setTimeout(resolve, pollIntervalMs)); + } + process.stdout.write('\n'); + throw new Error( + `RHDH did not become ready at ${url} within ${timeoutMs / 1000}s. ` + + `Run \`rhdh-cli plugin dev logs --rhdh\` to check for startup errors.`, + ); +} + const requiredRuntimeFiles = [ 'compose.yaml', 'compose-dynamic-plugins-root.yaml', @@ -69,8 +134,9 @@ export async function validateContainerTool( ); } // Verify the tool is actually on PATH before attempting any compose operation. + // Use execFile silently — a version check produces no useful output for the user. try { - await Task.forCommand(`${containerTool} --version`); + await execFile(containerTool, ['--version'], { shell: false }); } catch { throw new Error( `Unable to find ${containerTool} on PATH. Make sure ${containerTool} is installed and available, or pass --container-tool docker if you use Docker instead.`, @@ -87,10 +153,10 @@ async function resolveAndValidate(opts: OptionValues) { return { runtimeDir, containerTool }; } -async function getRuntimeStatus( +async function getComposeServices( containerTool: string, runtimeDir: string, -): Promise { +): Promise { const { stdout } = await execFile( containerTool, composeStatusArgs(containerTool), @@ -99,7 +165,38 @@ async function getRuntimeStatus( shell: false, }, ); - return formatRuntimeStatus(parseComposeStatus(stdout)); + return parseComposeStatus(stdout); +} + +async function getRuntimeStatus( + containerTool: string, + runtimeDir: string, +): Promise { + return formatRuntimeStatus( + await getComposeServices(containerTool, runtimeDir), + ); +} + +/** + * Throws unless the `rhdh` Compose service is currently running. + * + * `update`'s round-trip (re-export, re-stage, `compose start rhdh`) assumes + * the runtime was already brought up by `start`. Without this check, running + * `update`/`update --watch` against a runtime that was never started (or was + * stopped in another terminal) fails deep inside a raw `compose`/`container` + * subprocess error instead of a clear, actionable message. + */ +async function ensureRuntimeRunning( + containerTool: string, + runtimeDir: string, +): Promise { + const services = await getComposeServices(containerTool, runtimeDir); + const rhdh = findComposeService(services, 'rhdh'); + if (!composeServiceState(rhdh).includes('running')) { + throw new Error( + 'RHDH Local is not running. Run `rhdh-cli plugin dev start` first.', + ); + } } async function compose( @@ -110,18 +207,79 @@ async function compose( await run(containerTool, args, { cwd: runtimeDir, shell: false }); } +/** + * Wait for the `install-dynamic-plugins` container to finish, without + * needlessly stalling on a re-entrant `start` call. + * + * `waitForContainerEvent` subscribes to the container events stream starting + * *now* — it only sees events emitted after the subscription begins. `compose + * up -d` is idempotent and won't recreate/restart an already-exited one-shot + * container, so on a re-entrant `start` against an already-`Running` runtime, + * the installer's `died` event already happened in a prior invocation and + * will never be seen again. Without this check, that stalls `start` for the + * full `waitForContainerEvent` timeout (60s) before its timeout-resolve path + * lets it proceed anyway. + * + * Checking the installer's current Compose state first avoids that stall: + * if it already reports `exited`, the install already finished (in this run + * or a previous one) and there's nothing left to wait for. + */ +async function waitForInstallerToFinish( + containerTool: string, + runtimeDir: string, +): Promise { + const services = await getComposeServices(containerTool, runtimeDir); + const installer = findComposeService( + services, + 'install-dynamic-plugins', + 'rhdh-plugins-installer', + ); + if (composeServiceState(installer).includes('exited')) { + return; + } + // Podman's terminal container-death event is 'died'; Docker's is 'die'. + // Subscribing with the wrong filter means the event stream never matches + // anything and this stalls for the full waitForContainerEvent timeout. + const diedEvent = containerTool === 'podman' ? 'died' : 'die'; + await waitForContainerEvent( + containerTool, + 'install-dynamic-plugins', + diedEvent, + ); +} + export async function start(opts: OptionValues) { const { runtimeDir, containerTool } = await resolveAndValidate(opts); await validateProjectFiles(); await ensureGeneratedConfigIncluded(runtimeDir, opts.configure); + + Task.log('[1/4] Building and exporting plugin...'); await exportCommand({ build: true, install: true }); await stagePlugin(runtimeDir); + + Task.log('[2/4] Starting RHDH Local runtime...'); await compose(containerTool, runtimeDir, composeArgs('start')); - Task.log(await getRuntimeStatus(containerTool, runtimeDir)); + + Task.log('[3/4] Installing dynamic plugins...'); + await waitForInstallerToFinish(containerTool, runtimeDir); + + Task.log('[4/4] Waiting for RHDH to be ready...'); + const url = await resolveRhdhUrl(runtimeDir); + await waitForRhdhReady(url); + + Task.log(`\nRHDH is ready at ${url}`); + + if (opts.watch) { + await watchUpdate(containerTool, runtimeDir); + } } -export async function update(opts: OptionValues) { - const { runtimeDir, containerTool } = await resolveAndValidate(opts); +async function runUpdateCycle( + containerTool: string, + runtimeDir: string, + prefix = '', +): Promise { + await ensureRuntimeRunning(containerTool, runtimeDir); await validateProjectFiles(); // Fail fast if the generated config include is missing — without it, an // update re-stages the plugin but RHDH never loads it. Use configure: false @@ -132,7 +290,382 @@ export async function update(opts: OptionValues) { for (const action of ['install-dynamic-plugins', 'stop-rhdh', 'start-rhdh']) { await compose(containerTool, runtimeDir, composeArgs(action)); } - Task.log(await getRuntimeStatus(containerTool, runtimeDir)); + const url = await resolveRhdhUrl(runtimeDir); + await waitForRhdhReady(url); + Task.log(`${prefix}Refresh your browser at ${url}`); +} + +export async function update(opts: OptionValues) { + const { runtimeDir, containerTool } = await resolveAndValidate(opts); + // Always deploy the current tree immediately, matching `start`'s + // build-then-optionally-watch structure — `--watch` must not leave + // whatever's already on disk undeployed until the first change arrives. + // This also covers the ensureRuntimeRunning pre-flight check, so a + // stopped runtime fails fast here rather than only on the first + // change-triggered cycle inside watchUpdate. + await runUpdateCycle(containerTool, runtimeDir); + if (opts.watch) { + await watchUpdate(containerTool, runtimeDir); + } +} + +/** + * Container-events child processes (spawned by `waitForContainerEvent`) that + * are still running. Tracked so `watchUpdate`'s shutdown handler can kill + * them on SIGINT/SIGTERM instead of leaving them orphaned: `spawn()` puts + * children in the same process group as the parent by default, so a + * terminal Ctrl+C (which signals the whole foreground process group) takes + * them with it, but a direct `SIGTERM` to just this process's PID does not. + * Once `process.exit()` runs, this file's own JS-side timeout-driven + * `child.kill()` never gets a chance to fire either. + */ +const activeEventSubscriptions = new Set(); + +/** + * Subscribe to the container runtime event stream and resolve once a specific + * service emits a specific event action, or when `timeoutMs` elapses. + * + * Both Podman and Docker support `--format json` and `--filter label=`. + * Uses the `com.docker.compose.service` label set by Compose on all containers. + * + * @param containerTool - 'podman' or 'docker' + * @param service - Compose service name (e.g. 'install-dynamic-plugins') + * @param eventAction - Event action to wait for (e.g. 'died', 'cleanup', 'start') + * @param timeoutMs - Max wait in ms. 0 means no timeout (wait indefinitely). + */ +export async function waitForContainerEvent( + containerTool: string, + service: string, + eventAction: string, + timeoutMs = 60_000, +): Promise { + await new Promise(resolve => { + const child = spawn( + containerTool, + [ + 'events', + // Podman defaults to streaming but accepts (and needs, per its own + // docs) an explicit --stream. Docker's `events` has no such flag and + // always streams — passing it there makes Docker reject the command + // and exit immediately, so `close` would resolve this promise before + // any event is ever seen. + ...(containerTool === 'podman' ? ['--stream'] : []), + '--format', + 'json', + '--filter', + `event=${eventAction}`, + '--filter', + `label=com.docker.compose.service=${service}`, + ], + { stdio: ['ignore', 'pipe', 'ignore'] }, + ); + activeEventSubscriptions.add(child); + const finish = () => { + activeEventSubscriptions.delete(child); + resolve(); + }; + + const timer = + timeoutMs > 0 + ? setTimeout(() => { + child.kill(); + finish(); + }, timeoutMs) + : undefined; + + let buf = ''; + child.stdout.on('data', (chunk: Buffer) => { + buf += chunk.toString(); + const lines = buf.split('\n'); + buf = lines.pop() ?? ''; + for (const line of lines) { + const trimmed = line.trim(); + if (!trimmed) continue; + try { + const event = JSON.parse(trimmed) as { + Action?: string; + Status?: string; + status?: string; + Attributes?: Record; + // Docker nests the compose-service label here instead of at the + // top level (verified against Docker's documented events JSON + // schema); Podman uses the top-level `Attributes` above + // (verified against real `podman events --format json` output). + Actor?: { Attributes?: Record }; + }; + const action = event.Action ?? event.Status ?? event.status ?? ''; + const svc = + event.Attributes?.['com.docker.compose.service'] ?? + event.Actor?.Attributes?.['com.docker.compose.service'] ?? + ''; + if (action === eventAction && svc === service) { + if (timer !== undefined) clearTimeout(timer); + child.kill(); + finish(); + } + } catch { + // non-JSON line — ignore + } + } + }); + + child.on('error', err => { + if (timer !== undefined) clearTimeout(timer); + Task.log( + `Warning: could not watch for '${containerTool} events' (${describeWatchError(err)}). Proceeding without confirming ${service}'s '${eventAction}' event.`, + ); + finish(); + }); + + child.on('close', code => { + if (timer !== undefined) clearTimeout(timer); + // A null code means we killed it ourselves (SIGTERM) after already + // matching the event or hitting our own timeout — not a failure. + if (code !== null && code !== 0) { + Task.log( + `Warning: '${containerTool} events' exited unexpectedly (code ${code}). Proceeding without confirming ${service}'s '${eventAction}' event.`, + ); + } + finish(); + }); + }); +} + +/** + * Wait until both the `rhdh` and `install-dynamic-plugins` containers have + * emitted their terminal cleanup event, or until `timeoutMs` elapses. + * + * Podman: wait for `cleanup` (fires after `died`, once crun tears down exec.fifo). + * Docker: wait for `die` (terminal event; no equivalent cleanup step). + * + * Only used between back-to-back watch cycles to prevent the crun exec.fifo + * race condition. Single one-shot `plugin dev update` runs are unaffected. + */ +export async function waitForContainerCleanup( + containerTool: string, + timeoutMs: number, +): Promise { + // Podman emits `cleanup` after `died` once crun has torn down exec.fifo. + // Docker emits `die` as its terminal container event (no equivalent cleanup). + const settleEvent = containerTool === 'podman' ? 'cleanup' : 'die'; + + await Promise.all([ + waitForContainerEvent(containerTool, 'rhdh', settleEvent, timeoutMs), + waitForContainerEvent( + containerTool, + 'install-dynamic-plugins', + settleEvent, + timeoutMs, + ), + ]); +} + +const ignoredWatchSegments = new Set([ + 'node_modules', + 'dist', + 'dist-dynamic', + 'dist-types', +]); + +/** + * Predicate for chokidar's `ignored` option. + * + * Chokidar v4+ dropped glob-string support for `ignored` — it now only + * accepts a function, a regex, or a literal path. A glob-style array (e.g. + * double-star dist/node_modules patterns) is silently accepted but matches + * nothing, since chokidar no longer interprets `*` as a wildcard there. This + * checks path segments directly instead, so output directories are actually + * excluded rather than just documented as excluded. + * + * Chokidar always calls this with an absolute path. Segments are checked only + * within `root` (the plugin directory) — not the full absolute path — so a + * checkout that merely happens to live under an ancestor directory named + * e.g. `dist` or `node_modules` doesn't have every file ignored while still + * printing "Watching...". + */ +export function isIgnoredWatchPath(filePath: string, root: string): boolean { + const relative = path.relative(root, filePath); + // Outside root entirely — chokidar shouldn't call us with these for our + // watched paths, but stay conservative and never ignore what we can't + // place relative to the plugin. + if (relative.startsWith('..') || path.isAbsolute(relative)) return false; + return relative + .split(path.sep) + .some(segment => ignoredWatchSegments.has(segment)); +} + +/** + * Render a caught `unknown` value as a human-readable string for log output. + * + * Avoids `String(value)` on non-primitive values, which falls back to + * `Object.prototype.toString()` (`[object Object]`) for plain objects that + * don't define their own `toString()`. + */ +function describeWatchError(err: unknown): string { + if (err instanceof Error) return err.message; + if (typeof err === 'string') return err; + if ( + typeof err === 'number' || + typeof err === 'boolean' || + typeof err === 'bigint' + ) { + return String(err); + } + try { + return JSON.stringify(err) ?? 'Unknown error'; + } catch { + return 'Unknown error'; + } +} + +/** + * Watch source files and run a full update cycle on changes. + * + * Design: + * - Watches src/, package.json, and tsconfig.json. + * - Excludes output directories (node_modules, dist, dist-dynamic, + * dist-types) to prevent output-loop triggering. + * - Events are debounced: the first event in a `debounceMs` window triggers + * the cycle; subsequent events within the window are coalesced. + * - Cycles are serialized: if a cycle is already running when the debounce + * fires, the pending change is deferred until the active cycle finishes. + * - A failed cycle logs the error and continues watching; it does not exit. + * - SIGINT/SIGTERM cleanly close the watcher and exit. + * + * @param debounceMs - Milliseconds to wait after a change before triggering a + * cycle. Defaults to 500ms; callers may pass 0 for tests. + * @param settleTimeoutMs - Maximum milliseconds to wait for container cleanup + * events between back-to-back cycles. If the events don't arrive within this + * window the next cycle starts anyway. Defaults to 15000ms; callers may pass + * 0 to skip event-based settling entirely (used in tests). + */ +export async function watchUpdate( + containerTool: string, + runtimeDir: string, + debounceMs = 500, + settleTimeoutMs = 15_000, +): Promise { + const root = paths.targetDir; + const watchPaths = [ + paths.resolveTarget('src'), + paths.resolveTarget('package.json'), + // The only build-configuration file `plugin new` scaffolds (see + // adaptStandaloneProject); chokidar is fine watching a path that + // doesn't exist yet, so this is safe for projects without one. + paths.resolveTarget('tsconfig.json'), + ]; + + Task.log( + 'Watching for changes. Press Ctrl+C to stop.\n' + + ` Watching: src/, package.json, tsconfig.json\n` + + ` Ignored: node_modules/, dist/, dist-dynamic/, dist-types/`, + ); + + const watcher = chokidar.watch(watchPaths, { + ignored: filePath => isIgnoredWatchPath(filePath, root), + ignoreInitial: true, + persistent: true, + }); + + let running = false; + let pendingChange = false; + let debounceTimer: ReturnType | undefined; + + const drainCycles = async () => { + let keepGoing = true; + while (keepGoing) { + running = true; + pendingChange = false; + const cycleStart = Date.now(); + // Start listening for this cycle's own compose stop/install actions' + // terminal event *before* runUpdateCycle runs them below — + // waitForContainerEvent only sees events emitted after the + // subscription begins, so subscribing afterward (once the actions have + // already run) almost always misses the event and burns the full + // settleTimeoutMs on every queued follow-up cycle. Only awaited below + // if a follow-up cycle turns out to be needed; otherwise left to + // resolve on its own (killed on shutdown via activeEventSubscriptions + // if the process exits first). + const settlePromise = + settleTimeoutMs > 0 + ? waitForContainerCleanup(containerTool, settleTimeoutMs) + : undefined; + try { + Task.log(`\n[watch] Change detected — starting update cycle...`); + await runUpdateCycle(containerTool, runtimeDir, '[watch] '); + Task.log(`[watch] Update complete in ${Date.now() - cycleStart}ms.`); + } catch (err: unknown) { + const message = describeWatchError(err); + Task.log( + `[watch] Update failed after ${Date.now() - cycleStart}ms: ${message}`, + ); + Task.log('[watch] Watching for further changes...'); + } + keepGoing = pendingChange; + if (keepGoing) { + Task.log( + `[watch] Change received during cycle — waiting for runtime to settle...`, + ); + if (settlePromise !== undefined) await settlePromise; + } + // Cleared only now, after any settle-wait completes — not in a + // `finally` right after the cycle itself. Clearing it earlier let a + // change arriving during the settle-wait see `running === false` and + // start a second, concurrent drainCycles() instead of being coalesced + // into pendingChange for this same loop. + running = false; + } + }; + + const scheduleUpdate = () => { + if (debounceTimer !== undefined) return; // already scheduled + debounceTimer = setTimeout(async () => { + debounceTimer = undefined; + if (running) { + // A cycle is active — record the intent so the while loop in + // drainCycles picks up another cycle when the current one + // (including any settle wait) finishes. + pendingChange = true; + return; + } + await drainCycles(); + }, debounceMs); + }; + + watcher.on('all', (_event, filePath) => { + Task.log(`[watch] ${filePath} changed`); + scheduleUpdate(); + }); + + const shutdown = async () => { + Task.log('\n[watch] Shutting down...'); + if (debounceTimer !== undefined) { + clearTimeout(debounceTimer); + } + // Kill any in-flight `events` subscriptions (e.g. a pending settle-wait) + // explicitly. SIGINT gets these for free via the terminal's + // whole-process-group signal, but a direct SIGTERM to just this + // process's PID does not, and process.exit() below would otherwise + // leave them orphaned before their own JS-side timeout ever fires. + for (const child of activeEventSubscriptions) { + child.kill(); + } + await watcher.close(); + process.exit(0); + }; + + process.on('SIGINT', shutdown); + process.on('SIGTERM', shutdown); + + // Keep the process alive while the watcher is active. A fatal watcher + // error logs and ends the keep-alive promise. + await new Promise((_resolve, reject) => { + watcher.on('error', (err: unknown) => { + const message = describeWatchError(err); + Task.log(`[watch] Watcher error: ${message}`); + reject(err); + }); + }); } export async function stop(opts: OptionValues) { @@ -148,6 +681,7 @@ export async function stop(opts: OptionValues) { export async function restart(opts: OptionValues) { const { runtimeDir, containerTool } = await resolveAndValidate(opts); + await ensureRuntimeRunning(containerTool, runtimeDir); await compose(containerTool, runtimeDir, composeArgs('stop-rhdh')); await compose(containerTool, runtimeDir, composeArgs('start-rhdh')); Task.log(await getRuntimeStatus(containerTool, runtimeDir)); @@ -299,7 +833,8 @@ export async function ensureGeneratedConfigIncluded( if (alreadyIncluded) return; if (!configure) { throw new Error( - `Add ${generatedConfig} to ${override}'s includes list, or rerun with --configure.`, + `Add ${generatedConfig} to ${override}'s includes list, or run ` + + '`rhdh-cli plugin dev start --configure`.', ); } if (includes !== undefined && !YAML.isSeq(includes)) { @@ -410,25 +945,43 @@ export function parseComposeStatus(output: string): ComposeService[] { }); } +function composeServiceNames(item: ComposeService): string[] { + return Array.isArray(item.Names) + ? item.Names + : (item.Names?.split(',') ?? []); +} + +function findComposeService( + services: ComposeService[], + ...serviceNames: string[] +): ComposeService | undefined { + return services.find( + item => + serviceNames.includes(item.Service ?? '') || + serviceNames.includes(item.Name ?? '') || + composeServiceNames(item).some(name => serviceNames.includes(name)), + ); +} + +function composeServiceState(item: ComposeService | undefined): string { + return (item?.State ?? item?.Status ?? '').toLowerCase(); +} + +function composeServiceExitCode( + item: ComposeService | undefined, +): string | undefined { + return item?.ExitCode === undefined ? undefined : String(item.ExitCode); +} + export function formatRuntimeStatus(services: ComposeService[]): string { - const names = (item: ComposeService) => - Array.isArray(item.Names) ? item.Names : (item.Names?.split(',') ?? []); - const service = (...serviceNames: string[]) => - services.find( - item => - serviceNames.includes(item.Service ?? '') || - serviceNames.includes(item.Name ?? '') || - names(item).some(name => serviceNames.includes(name)), - ); - const rhdh = service('rhdh'); - const installer = service( + const rhdh = findComposeService(services, 'rhdh'); + const installer = findComposeService( + services, 'install-dynamic-plugins', 'rhdh-plugins-installer', ); - const state = (item: ComposeService | undefined) => - (item?.State ?? item?.Status ?? '').toLowerCase(); - const exitCode = (item: ComposeService | undefined) => - item?.ExitCode === undefined ? undefined : String(item.ExitCode); + const state = composeServiceState; + const exitCode = composeServiceExitCode; if ( state(installer).includes('exited') && diff --git a/src/commands/index.ts b/src/commands/index.ts index 272446d..1cb64d2 100644 --- a/src/commands/index.ts +++ b/src/commands/index.ts @@ -227,12 +227,20 @@ export function registerPluginCommand(program: Command) { '--configure', 'Add the CLI-managed plugin configuration include to RHDH Local on first use', ) + .option( + '--watch', + 'After startup, watch source files and re-run the update cycle automatically on change', + ) .action(lazy(() => import('./dev').then(m => m.start))); devSharedOptions(dev.command('update')) .description( 'Re-export and re-stage the plugin, then restart the RHDH service', ) + .option( + '--watch', + 'Watch source files and re-run the update cycle automatically on change', + ) .action(lazy(() => import('./dev').then(m => m.update))); devSharedOptions(dev.command('stop')) diff --git a/src/commands/new/__snapshots__/command.test.ts.snap b/src/commands/new/__snapshots__/command.test.ts.snap index 869d678..5295e25 100644 --- a/src/commands/new/__snapshots__/command.test.ts.snap +++ b/src/commands/new/__snapshots__/command.test.ts.snap @@ -99,7 +99,8 @@ npx @red-hat-developer-hub/cli plugin export "packageManager": "yarn@4.17.1", "version": "0.1.0", "resolutions": { - "@types/express": "4.17.21" + "@types/express": "4.17.21", + "@backstage/cli-defaults": "0.1.5" } } ", @@ -297,7 +298,8 @@ npx @red-hat-developer-hub/cli plugin export "packageManager": "yarn@4.17.1", "version": "0.1.0", "resolutions": { - "@types/express": "4.17.21" + "@types/express": "4.17.21", + "@backstage/cli-defaults": "0.1.5" } } ", @@ -462,7 +464,8 @@ npx @red-hat-developer-hub/cli plugin export "packageManager": "yarn@4.17.1", "version": "0.1.0", "resolutions": { - "@types/express": "4.17.21" + "@types/express": "4.17.21", + "@backstage/cli-defaults": "0.1.5" } } ", diff --git a/src/commands/new/command.test.ts b/src/commands/new/command.test.ts index 2900857..841f829 100644 --- a/src/commands/new/command.test.ts +++ b/src/commands/new/command.test.ts @@ -124,7 +124,9 @@ describe('createPluginProject', () => { ); expect(packageJson.devDependencies.typescript).toBe('5.4.5'); expect(packageJson.resolutions['@types/express']).toBe('4.17.21'); + expect(packageJson.resolutions['@backstage/cli-defaults']).toBe('0.1.5'); expect(Object.keys(packageJson.resolutions).sort()).toEqual([ + '@backstage/cli-defaults', '@types/express', ]); expect( diff --git a/src/commands/new/rhdhProfiles.ts b/src/commands/new/rhdhProfiles.ts index 806e734..54f74bf 100644 --- a/src/commands/new/rhdhProfiles.ts +++ b/src/commands/new/rhdhProfiles.ts @@ -34,6 +34,9 @@ export const rhdhProfiles: Record = { // transitives follow the ranges published by upstream packages. // Newer express type packages are incompatible with the RHDH 2.1 toolchain. '@types/express': '4.17.21', + // Pin cli-defaults to prevent transitive resolution of 0.1.6+ which pulls in + // cli-module-package-manager-yarn with broken internal yarn patches. + '@backstage/cli-defaults': '0.1.5', }, // Keep frontend development dependencies out of backend and module projects. templateRoleOverlays: { diff --git a/yarn.lock b/yarn.lock index 4a1f447..91bdfcd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3418,6 +3418,7 @@ __metadata: "@yarnpkg/parsers": "npm:^3.0.0-rc.4" axios: "npm:^1.9.0" chalk: "npm:^4.0.0" + chokidar: "npm:^5.0.0" codeowners: "npm:^5.1.1" commander: "npm:^9.1.0" eslint: "npm:8.57.1" @@ -6900,6 +6901,15 @@ __metadata: languageName: node linkType: hard +"chokidar@npm:^5.0.0": + version: 5.0.0 + resolution: "chokidar@npm:5.0.0" + dependencies: + readdirp: "npm:^5.0.0" + checksum: 10/a1c2a4ee6ee81ba6409712c295a47be055fb9de1186dfbab33c1e82f28619de962ba02fc5f9d433daaedc96c35747460d8b2079ac2907de2c95e3f7cce913113 + languageName: node + linkType: hard + "chownr@npm:^1.1.1": version: 1.1.4 resolution: "chownr@npm:1.1.4" @@ -15185,6 +15195,13 @@ __metadata: languageName: node linkType: hard +"readdirp@npm:^5.0.0": + version: 5.1.1 + resolution: "readdirp@npm:5.1.1" + checksum: 10/4a6a61272c6db332db361166b42be34ce8480542073061f04f405ed2f703e517e90dfafe33843f647e91d20090c575aee04a823e9ed9d9d9a01ec3603ca45a8b + languageName: node + linkType: hard + "readdirp@npm:~3.6.0": version: 3.6.0 resolution: "readdirp@npm:3.6.0"