Conversation
Implements `rhdh-cli plugin dev [start|stop|update|logs|status]` to manage a plugin's lifecycle against an existing RHDH Local checkout. - start: exports and stages the plugin into local-plugins/, writes rhdh-cli.generated.yaml, and brings up the Compose runtime - update: re-exports, re-stages, restarts the installer and RHDH service without tearing down the full runtime - stop: stops Compose services; --clean also removes containers and networks (volumes and config are preserved) - logs: streams Compose logs; --installer shows installer output, --rhdh shows RHDH output, --all includes all services - status: interprets installer exit code and RHDH service state into a human-readable status message - --configure: non-invasively adds the CLI-managed config include to dynamic-plugins.override.yaml on first use Staging uses fs.remove + dereference:false copy to handle relative symlinks in node_modules/.bin without errors on repeated runs. Also fixes two issues discovered during live testing: - plugin new: inject version '0.1.0' in adaptStandaloneProject so upstream templates (which omit version) produce packages that npm pack can handle in the RHDH Local installer - plugin export: add preflight version checks in both backend.ts and frontend.ts that fail fast with a clear message when package.json is missing a version field, rather than letting npm pack fail inside the installer container with a cryptic error - plugin export: ensureDir before writing config schema in command.ts Assisted-By: OpenCode Signed-off-by: Stan Lewis <[email protected]> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 7:05 PM UTC · Completed 7:31 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-4-6 · Effort: high · Cost: $6.42 |
|
Risk Assessment: moderate (2/5) DetailsTier 1 and Tier 2 signals are unchanged from the prior review — the same large blast radius (12 files, 1008 lines) is offset by zero security-sensitive paths, no CI or dependency changes, and a non-first-time human author; the only delta since the prior review is bug-fix commits on the new dev/ files which still carry no git history, leaving the composite at 2/moderate. Previous runRisk Assessment: moderate (2/5) DetailsTier 1 sub-score ~2.1 (weighted 62%): large blast radius and 989 lines pull upward, but no security-sensitive paths, no CI changes, no dependency files, and a normal human non-first-time author profile keep the average low. Tier 2 sub-score ~2 (weighted 38%): three new dev/ files have no prior history; export-dynamic-plugin files show modest fix activity but no regressions against the PRs changed lines. No Tier 3 (Jira issue not accessible). Composite rounds to 2, unchanged from prior review. Previous run (2)Risk Assessment: moderate (2/5) DetailsScore drops from prior 3 to 2 because Tier 2 history is clean — the three new dev/ files have no regression history, zero reverts across all changed files, and low average churn — partially offset by large blast radius and below-threshold test-file ratio. Previous run (3)Risk Assessment: moderate (2/5) DetailsModerate risk: a well-structured new feature addition by an experienced contributor with unit tests and no dependency or CI changes, offset by a large blast radius classification and modifications to export-dynamic-plugin files that carry significant fix/revert history. |
ReviewFindingsMedium
Low
Next steps:
Previous runReviewFindingsMedium
Low
Next steps:
Previous run (2)ReviewFindingsMedium
Low
Next steps:
Previous run (3)ReviewFindingsMedium
Low
Next steps:
|
- Add Apache 2.0 license headers to command.ts and index.ts
- Move YAML import to third-party import group
- Use replaceAll('/', '-') in stagePlugin for complete scope segment replacement
- Filter non-JSON lines and add try/catch in parseComposeStatus
- Log a warning in updateGeneratedConfig when overwriting a different plugin entry
- Add --follow flag to plugin dev logs for streaming output
- Add CHANGELOG entries for plugin dev command and breaking plugin export changes
Assisted-By: claude-sonnet-4-6@default
rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
gashcrumb
left a comment
There was a problem hiding this comment.
Addressed all fullsend findings in the follow-up commit b9e8f9b. Summary below; inline comments cover each thread.
Fixed: license headers, YAML import order, replaceAll, parseComposeStatus hardening, overwrite warning, --follow flag for logs, CHANGELOG entries.
Dismissed: the actionsToRun edge-case note (functional by design — composeArgs throws a clear error for unknown actions).
…tory SonarCloud flagged fs.chmod(destination, 0o755) as setting world-accessible permissions. The chmod was applied only to the top-level directory and was speculative — fs.copy preserves source permissions from dist-dynamic, which the export process already creates with appropriate modes. Remove it. Assisted-By: claude-sonnet-4-6@default rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
Assisted-By: claude-sonnet-4-6@default rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
- Use node:path import prefix (typescript:S7772) - Extract nested template literal expressions into local variables in formatRuntimeStatus (typescript:S4624, lines 350/356/359) Assisted-By: claude-sonnet-4-6@default rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
Run lint, prettier, and tsc before pushing to a PR branch. Assisted-By: claude-sonnet-4-6@default rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
…bcommands Each plugin dev action (start, update, stop, logs, status) is now a proper Commander subcommand with only the flags that apply to it: - start: --rhdh-local-dir, --container-tool, --configure - update: --rhdh-local-dir, --container-tool - stop: --rhdh-local-dir, --container-tool, --clean - logs: --rhdh-local-dir, --container-tool, --rhdh, --installer, --all, --follow - status: --rhdh-local-dir, --container-tool This makes plugin dev -h list all available subcommands and prevents nonsensical option combinations like plugin dev status --configure or plugin dev start --clean. command.ts exports one handler per subcommand plus all shared helpers. actionsToRun() is removed; each handler directly invokes the compose actions it needs. AGENTS.md documents the subcommand convention so future command groups follow the same pattern. Assisted-By: claude-sonnet-4-6@default rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 1:46 PM UTC · Completed 2:05 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-4-6 · Effort: high · Cost: $4.22 |
In a Backstage monorepo workspace the tsconfig.json sits at the workspace root (e.g. workspaces/todo/) with outDir: dist-types and rootDir: '.', so tsc writes types to <workspaceRoot>/dist-types/<relative-plugin-path>/ rather than inside the plugin directory itself. validateProjectFiles() was always checking <pluginDir>/dist-types, which never exists in a monorepo. Introduce resolveDistTypes() which discriminates on paths.targetDir vs paths.targetRoot: - targetDir === targetRoot (standalone): dist-types is inside the plugin dir - targetDir !== targetRoot (monorepo workspace): dist-types is at <targetRoot>/dist-types/<relative-path-from-root-to-plugin> Three new tests cover: standalone resolution, monorepo path calculation, and the full validateProjectFiles() flow against a simulated monorepo layout. Assisted-By: claude-sonnet-4-6@default rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
- composeArgs: replace four positional boolean params with a LogsOptions object for call-site clarity; update all callers and tests - composeStatusArgs: pass --all unconditionally (both Docker Compose and Podman Compose >=1.3 accept it); fixes potential misreporting on Podman when the installer container exits before status is queried - parseComposeStatus: wrap the array-format JSON.parse branch in try/catch, consistent with the JSON Lines branch - stagePlugin: guard against a crafted package name (e.g. '..') resolving the destination outside local-plugins/ before calling fs.remove - import order: move node:path below third-party imports, matching sibling files that give Node built-ins their own group - Remove box-drawing section separator comments; use plain blank lines between logical groups to stay consistent with the rest of src/commands/ - README: document that the default container tool is podman and how to switch to docker Assisted-By: claude-sonnet-4-6@default rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
…setup SonarCloud reported 3.4% duplicated lines on the PR (threshold: 3%) due to near-identical setup/teardown in the two stagePlugin tests. Extract a shared withStagingDirs() helper that creates srcDir/runtimeDir, writes package.json, sets the test global, and handles cleanup in a finally block. Assisted-By: claude-sonnet-4-6@default rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 2:50 PM UTC · Completed 3:08 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-4-6 · Effort: high · Cost: $5.14 |
- stagePlugin: extend path-traversal guard to also reject empty or
identity-resolving relative paths (e.g. package name '@' strips to '',
which path.relative returns as '' — the same issue as '.'). Guard now
checks !relative || relative === '.' in addition to the existing
startsWith('..') and isAbsolute checks. Test covers both '..' and '@'.
- formatRuntimeStatus: add undefined check before comparing exitCode to
'0'. Previously an exited installer container with no ExitCode field
(exitCode returns undefined) would satisfy undefined !== '0' and be
misreported as a plugin installation failure. Guard is now:
exitCode !== undefined && exitCode !== '0'. Test covers this case.
Assisted-By: claude-sonnet-4-6@default
rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 4:49 PM UTC · Completed 5:07 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-4-6 · Effort: high · Cost: $4.72 |
There was a problem hiding this comment.
Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:
README.md(file-level): Line 128 · [medium] stale-doc
The ## Commands → ### Plugin Development Commands bullet list still omits plugin dev. The PR added a prose section documenting usage but the command index was not updated. A reader consulting the Commands section will find no entry for the new subcommand group (start, update, stop, logs, status).
Suggested fix: Add a bullet to the Plugin Development Commands list: - \plugin dev`: Export a dynamic plugin and manage its lifecycle against an existing RHDH Local runtime (`start`, `update`, `stop`, `logs`, `status`)`
…uacy restart: - New 'plugin dev restart' subcommand stops and restarts the RHDH service without re-exporting or re-staging the plugin. Useful when changing RHDH Local configuration (app-config, override YAML, etc.) without touching plugin code. 'plugin dev update' remains the command for plugin code changes. - Registered in src/commands/index.ts alongside the other dev subcommands. - Documented in README, CHANGELOG, and AGENTS.md. stale-doc (fullsend): - Add 'plugin dev' bullet (including 'restart') to the Commands index in README. - Update CHANGELOG and README prose to include 'restart' in the subcommand list. test-adequacy (fullsend): - Add test for parseComposeStatus JSON array branch (Podman Compose format). - Add test for malformed JSON array input triggering the try/catch error path. Assisted-By: claude-sonnet-4-6@default rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
Addressed in [stale-doc] Added [test-adequacy] Added two [protected-path] The AGENTS.md changes are intentional — they document the [feat] Also added |
podman-compose ps does not support --all and exits with an error when it is passed. The earlier change to pass --all unconditionally broke plugin dev status and plugin dev restart for podman users. Restore the original discriminator: --all is added only for docker, where compose ps requires it to include exited containers. podman-compose includes exited containers by default (verified with 1.6.0) so the flag is omitted. composeStatusArgs() takes containerTool again. Update the call site in getRuntimeStatus and add a test for the podman variant alongside the existing docker test. Assisted-By: claude-sonnet-4-6@default rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|



Summary
Implements
rhdh-cli plugin dev(RHIDP-16672) — a command that exports a dynamic plugin and manages its lifecycle against an existing RHDH Local checkout.Commands
plugin dev startplugin dev updateplugin dev stop--cleanalso removes containers and networksplugin dev logs--rhdhfor RHDH output,--installerfor installer outputplugin dev status--rhdh-local-dir <path>orRHDH_LOCAL_DIRpoints at an existing RHDH Local checkout.--configurenon-invasively adds the CLI-managed config include todynamic-plugins.override.yamlon first use.Runtime contract
local-plugins/<name>/for the RHDH Local installer'snpm packpathconfigs/dynamic-plugins/rhdh-cli.generated.yamlstop --cleanusescompose down— volumes and config are always preservedAlso fixes
plugin new: inject"version": "0.1.0"inadaptStandaloneProject— upstream standalone templates omitversion, butnpm pack(used by the RHDH Local installer) requires itplugin export: preflight check in bothbackend.tsandfrontend.tsthat fails fast with a clear message whenpackage.jsonis missing aversionfield, rather than lettingnpm packfail inside the installer containerplugin export:ensureDirbefore writing the config schema incommand.tsstagePluginusesfs.remove+fs.copy({ dereference: false })to preserve relative symlinks innode_modules/.bin/and avoid self-copy errors on repeatedupdaterunsTesting
plugin dev start→plugin dev update→plugin dev stopagainst a real RHDH Local instance runningquay.io/rhdh-community/rhdh:nextensureGeneratedConfigIncluded,updateGeneratedConfig,stagePluginsymlink handling,validateProjectFiles(backend requiresdist-types, frontend does not), andformatRuntimeStatusCloses #RHIDP-16672