feat: add plugin upgrade command for dynamic plugin dependency management

[gashcrumb]

Description

Implements RHIDP-16666 (rhdh-cli plugin upgrade command) under epic RHIDP-13609 / RHDHPLAN-985.

Note: This PR builds upon the version resolution engine introduced in PR #176. Once #176 merges into main, this branch will cleanly contain only the plugin upgrade additions.

Key Features:

1. Plugin Upgrade Command (src/commands/upgrade/):

   • Adds rhdh-cli plugin upgrade [rhdhVersion] and alias rhdh-cli plugin versions:bump.
   • Automatically aligns all @backstage/* dependencies across dependencies, devDependencies, and peerDependencies in package.json to match the target release manifest.
   • Preserves range specifiers (^, ~) and exact version pins, while leaving third-party / non-manifest dependencies untouched.
   • Synchronizes backstage.json version when present.
   • Warns on unmanifested @backstage/* packages.
   • Detects Yarn (yarn.lock) or npm and runs install to synchronize lockfiles (unless --skip-install is passed).

2. Command Flags:

   • --dry-run: Displays planned package updates in a formatted table without writing to disk.
   • --skip-install: Skips running package manager install after updating package.json.
   • --manifest-file <path>: Path to local Backstage manifest for offline/air-gapped environments.
   • --json: Machine-readable output for automation scripts.

Jira Issues:

• https://redhat.atlassian.net/browse/RHIDP-16666
• https://redhat.atlassian.net/browse/RHIDP-13609
• https://redhat.atlassian.net/browse/RHDHPLAN-985

Verification:

• Unit tests added covering target version computation, package manager detection, dry-run mode, install behavior, and error handling (yarn test).
• Typechecking (yarn tsc), linting (yarn lint:check), and formatting (yarn prettier:check) passing.

[PatAKnight]

Couple of comments

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:42 PM UTC · Completed 12:57 PM UTC

Commit: 7ed3578 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.53

[fullsend-ai-review]

Risk Assessment: moderate (2/5)

Details

Tier 1 signals unchanged from prior run (large blast radius from 924-line addition, zero protected paths, no security-sensitive or CI changes, one dependency file, 0.11 test ratio, known contributor); Tier 2 shows moderate churn on package.json and CHANGELOG but all recent (<7d), keeping the composite at 2.10 — consistent with prior score of 2 (moderate).

Previous run

Risk Assessment: moderate (2/5)

Details

Composite of 0.62x2.13 + 0.38x1.33 = 1.82 rounds to 2; the large blast radius and 890-line addition drive Tier 1 change-size to 5 but are offset by zero protected paths, no security-sensitive or CI changes, one dependency file, partial test coverage (ratio 0.11), and a known contributor — Tier 2 confirms low churn on the new core files — yielding moderate risk consistent with the prior score of 2.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Large blast radius from 883 lines and 8 files but no protected paths, security-sensitive files, CI workflow changes, or dependency modifications; test coverage present (ratio 0.12) and known contributor; low Tier 2 churn on core new files yields composite ~1.9, rounding to 2, consistent with the prior moderate rating across all review runs.

Previous run (3)

Risk Assessment: moderate (2/5)

Details

Tier 1 signals unchanged from prior assessment — large blast radius from 871 lines and 8 files but no protected paths, security-sensitive files, CI workflow changes, or dependency modifications, with test coverage present and a known contributor; Tier 2 history is stable with low churn, yielding a composite of 1.73 (62%×1.875 + 38%×1.5), which rounds to 2, consistent with the prior moderate rating.

Previous run (4)

Risk Assessment: moderate (2/5)

Details

Tier 1 signals unchanged from prior assessment: large blast radius from 854 lines and 4 new source files, but no protected paths, security-sensitive files, CI workflow changes, or dependency modifications, and test coverage is present with a known contributor, yielding a composite of 1.81 rounded to 2.

Previous run (5)

Risk Assessment: moderate (2/5)

Details

Tier 1 signals unchanged from prior assessment; blast radius large but mitigated by most files being new. No protected paths, security-sensitive files, CI workflows, or dependency files touched. Test coverage present (0.12 ratio). Known contributor. Composite 2.0 preserves prior moderate rating.

Previous run (6)

Risk Assessment: moderate (2/5)

Details

New feature adding a plugin upgrade command across 5 files (714 lines). Blast radius is large but mitigated by 3 of 5 files being entirely new with no impact on existing code paths. No protected paths, security-sensitive files, CI workflows, or dependency files touched. Test coverage present (0.20 file ratio). Known contributor. Moderate churn on existing files follows expected patterns.

[fullsend-ai-review]

Looks good to me

Previous run

Review

Findings

Medium

• [code-organization] src/commands/upgrade/command.ts:365 — printUpgradeResult is documented as a rendering function (JSDoc: "Renders human-readable results") with return type void, but it throws ExitCodeError(1) in the installFailed branch. The sibling check-versions/command.ts establishes the pattern that throws belong exclusively in the command entry point, keeping render helpers side-effect-free. A caller reading printUpgradeResult's signature has no indication it can propagate a process-exiting error. The command function already handles installFailed via throw new ExitCodeError(1) for the JSON output path (line 402), making the split inconsistent even within the same file.
  Remediation: Return a sentinel from printUpgradeResult (e.g. return true when installFailed) and move throw new ExitCodeError(1) into command, mirroring the check-versions pattern where command owns all exit-code-bearing throws.

Low

• [edge-case] src/commands/upgrade/command.ts:79 — The regex /^(?:\^|~|>=|<=|>|<|=)/ correctly handles single-operator range prefixes but silently mangles compound semver ranges (e.g. >=1.0.0 <2.0.0 becomes >=<manifestVersion>, dropping the upper-bound constraint). Practically benign — standard Backstage plugin dependencies use ^, ~, or exact pins, not compound ranges, and the test suite covers all expected forms.

• [error-handling] src/commands/upgrade/command.ts:189 — syncBackstageJson silently swallows JSON parse errors (the catch block comments // Ignore JSON read errors and returns undefined). A user with a malformed backstage.json gets no warning that the file was skipped while package.json was upgraded. Appears to be a deliberate design choice for an optional file; a debug-level log would improve observability.

• [naming-convention] src/commands/upgrade/command.ts:38 — The public interface names UpgradePluginOptions and UpgradePluginResult are derived from the internal helper function name upgradePluginDependencies rather than from the command name. The sibling command establishes a <CommandName>Options / <CommandName>Result pattern (check-versions → CheckVersionsOptions / CheckVersionsResult). The parallel convention for this command would be UpgradeOptions / UpgradeResult.

• [missing-authorization] N/A — The PR references internal Jira tickets only (RHIDP-16666, RHIDP-13609, RHDHPLAN-985), which are not publicly verifiable. The pattern is consistent with the team's accepted workflow — PR feat: add plugin check-versions command and RHDH version resolution engine #176 (the companion check-versions command) was merged under identical authorization conditions. Noted for transparency only.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (2)

Review

Findings

Low

• [edge-case] src/commands/upgrade/command.ts:79 — The regex /^(?:\^|~|>=|<=|>|<|=)/ correctly handles single-operator range prefixes but silently mangles compound semver ranges (e.g. >=1.0.0 <2.0.0 becomes >=<manifestVersion>, dropping the upper-bound constraint). Practically benign — standard Backstage plugin dependencies use ^, ~, or exact pins, not compound ranges, and the test suite covers all expected forms.

• [error-handling] src/commands/upgrade/command.ts:189 — syncBackstageJson silently swallows JSON parse errors (the catch block comments // Ignore JSON read errors and returns undefined). A user with a malformed backstage.json gets no warning that the file was skipped while package.json was upgraded. This appears to be a deliberate design choice for an optional file; a debug-level log would improve observability.

• [missing-authorization] N/A — This feature addition (400+ lines, two new source files) references only internal Jira tickets (RHIDP-16666, RHIDP-13609, RHDHPLAN-985), which are not publicly verifiable. The companion PR feat: add plugin check-versions command and RHDH version resolution engine #176 followed the same pattern and was merged, suggesting Jira-only linking is the team’s accepted workflow. Noted for transparency.

Info

• [scope-alignment] src/lib/pluginDependencies.ts — The DependencySection type is extracted from src/commands/check-versions/command.ts into a new shared module and re-imported by both commands. Architecturally coherent: placing cross-command shared types in src/lib/ matches the project’s existing pattern for paths, tasks, errors, and rhdhVersion.

Previous run (3)

Review

Findings

Low

• [edge-case] src/commands/upgrade/command.ts:68 — computeTargetVersion now correctly handles ^, ~, >=, <=, >, <, =, and workspace: range prefixes; the prior specifier-handling concern is resolved. A narrow residual case remains: workspace:1.0.0 (workspace protocol with an exact version pin, no range operator) falls through to the unchanged-return path and would not be bumped. This is practically benign since workspace: with exact pins is not a conventional Backstage dependency pattern and leaving it unchanged is safe.

• [documentation-comment-format] src/commands/upgrade/command.test.ts:124 — Test description reads 'preserves carat prefix' but ^ is a caret (not a carat, which measures gemstone weight). All other test descriptions in this file use precise terminology.
  Remediation: Change 'preserves carat prefix' to 'preserves caret prefix'.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (4)

Review

Findings

Low

• [edge case - range specifier handling] src/commands/upgrade/command.ts:59 — computeTargetVersion handles ^, ~, exact pins, and backstage:^ but silently treats any other range specifier (>=, <=, *, workspace:, etc.) as an exact pin, stripping the prefix. For example, >=1.0.0 would become 1.12.0 instead of >=1.12.0. While @backstage/* packages in dynamic plugins conventionally use only ^/~/exact/backstage:^, a user with a non-standard specifier would lose it without warning.
  Remediation: Consider logging a warning when the current version uses an unrecognized range prefix, or preserving arbitrary single-character prefixes.

• [display logic error] src/commands/upgrade/command.ts:319 — The table separator width is calculated as header.length + ' Status'.length, but the header string already ends with Status. This makes the separator 8 characters wider than the header and data rows.

• [documentation-comment-format] src/commands/upgrade/command.ts:272 — The private function printUpgradeResult is the only non-trivial function in the file that lacks a JSDoc comment. Every other helper function — applyDependencyUpgrades, syncBackstageJson, runInstallDependencies — carries a single-line JSDoc.
  Remediation: Add a brief JSDoc comment above the function declaration.

• [import-specifier-style] src/commands/upgrade/command.test.ts:18 — import os from 'os' uses the bare specifier instead of the node: protocol prefix. The same test file already uses 'node:path' on line 19, making the bare 'os' inconsistent within the same import block.
  Remediation: Change import os from 'os' to import os from 'node:os'.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (5)

Review

Findings

Medium

• [error handling / silent failure] src/commands/upgrade/command.ts:178 — The catch block in syncBackstageJson is labeled 'Ignore JSON read errors' but also silently swallows write errors from fs.writeJson on line 175. If backstage.json is readable but not writable (e.g., permissions, disk full), the function returns undefined, making the caller believe no update was needed. The result: package.json dependencies are upgraded but backstage.json is silently left at the old Backstage version, with no warning to the user.
  Remediation: Narrow the try/catch to only cover the fs.readJson call (or re-throw non-parse errors). Alternatively, move the fs.writeJson call outside the try/catch so write failures propagate to the caller.

Low

• [incomplete dry-run preview] src/commands/upgrade/command.ts:225 — In dry-run mode, the entire file-write block (lines 225–242) is skipped, including the syncBackstageJson call. The dry-run output reports only package.json dependency changes but does not indicate that backstage.json would also be updated. A user relying on dry-run to preview all modifications will not see the backstage.json version bump.
  Remediation: Add a note to the dry-run output listing backstage.json as a file that would be modified, or compute the backstage.json change before the dry-run guard.

• [import-ordering] src/commands/check-versions/command.ts:23 — The DependencySection import from ../../lib/pluginDependencies is placed before the ExitCodeError import from ../../lib/errors, breaking alphabetical order by module path. The sibling file upgrade/command.ts has these same imports in correct alphabetical order.
  Remediation: Move the DependencySection import after the paths import to restore alphabetical order.

• [declaration-spacing] src/commands/check-versions/command.ts:34 — The blank line that previously separated the type declarations from PackageCheckResult was lost when the DependencySection type block was extracted. DependencyStatus (ending at line 33) now runs directly into export interface PackageCheckResult (line 34) with no separator.
  Remediation: Add a blank line between line 33 and line 34.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (6)

Review

Findings

High

• [error handling / exit code contract] src/commands/upgrade/command.ts:353 — The install-failure path sets process.exitCode = 1 but does not throw. The lazy wrapper in src/commands/index.ts unconditionally calls process.exit(0) after the action function returns, which overrides any previously set process.exitCode. In production, an install failure will exit with code 0 (success), not 1. The sibling check-versions command correctly signals failure by throwing new ExitCodeError(1). The existing test passes only because it calls command() directly, bypassing the lazy wrapper. This also represents a deviation from the codebase's established error-handling idiom where every other command uses throw new ExitCodeError(code).
  Remediation: Replace process.exitCode = 1 with throw new ExitCodeError(1) (importing ExitCodeError from ../../lib/errors). Update the install-failure test to assert that the command rejects with ExitCodeError.

Medium

• [logic error / suppressed output] src/commands/upgrade/command.ts:287 — The early return on result.changes.length === 0 skips the unmanifested-packages warning block (lines 336–342). If every @backstage/* dependency is unmanifested (none appear in the release manifest), the command prints "No @backstage dependencies found to upgrade." and returns, silently dropping the unmanifested warning. The JSON output path is unaffected.
  Remediation: Move the unmanifested warning before the changes.length === 0 guard, or extend the guard to also check result.unmanifested.length === 0 before returning early.

• [missing-doc] README.md:43 — The PR adds a new public CLI command plugin upgrade (alias plugin versions:bump) with options --dry-run, --skip-install, --manifest-file, and --json, but README.md does not document it. The sibling command plugin check-versions has its own section with usage examples and option descriptions.
  Remediation: Add a new section to README.md after the "Checking Plugin Versions" section covering basic usage, the alias, and the four options.

Low

• [duplicate data] src/commands/upgrade/command.ts:134 — In applyDependencyUpgrades, the unmanifested array is populated for every dependency section a non-manifest @backstage/* package appears in, producing duplicates if the same package is in multiple sections.
  Remediation: Deduplicate unmanifested before returning.

• [type-duplication] src/commands/upgrade/command.ts:27 — The DependencySection type is defined identically in both check-versions/command.ts and upgrade/command.ts.
  Remediation: Consider extracting to a shared module.

• [cli-interface-consistency] src/commands/index.ts:167 — The upgrade command accepts the RHDH version as a positional argument (upgrade [rhdhVersion]), while check-versions uses a named option (--rhdh-version <version>).
  Remediation: Consider accepting both forms for consistency.

• [API shape / options interface] src/commands/upgrade/command.ts:45 — UpgradePluginOptions includes json?: boolean but upgradePluginDependencies never reads it; json is a CLI presentation concern.
  Remediation: Remove json?: boolean from UpgradePluginOptions.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

[gashcrumb]

Hmm, looks like the dynamic-home-page entry in the homepage workspace was inadvertently added back here by the overlay discovery workflow which is causing the homepage test to fail. Will have to look at why that was redicsovered as the re-introduction of this wasn't caught when merging the automatically created PR looks like, created RHDHBUGS-3746 to track it.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:24 PM UTC · Completed 1:42 PM UTC

Commit: 8cd84c1 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.10

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 1:59 PM UTC · Ended 2:08 PM UTC

Commit: ece9088 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:09 PM UTC · Completed 2:26 PM UTC

Commit: 2dc1563 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.03

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 2:52 PM UTC · Ended 3:00 PM UTC

Commit: 449d2cd · View workflow run →

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 3:01 PM UTC · Ended 3:09 PM UTC

Commit: 1b54744 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:10 PM UTC · Completed 3:29 PM UTC

Commit: f0787d3 · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-4-6 · Effort: high · Cost: $4.63

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:03 PM UTC · Completed 4:18 PM UTC

Commit: 99ac494 · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-4-6 · Effort: high · Cost: $3.35

Superseded by updated review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:57 AM UTC · Completed 12:13 PM UTC

Commit: eb67054 · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-4-6 · Effort: high · Cost: $3.69

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:32 PM UTC · Completed 5:48 PM UTC

Commit: fd1c54c · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-4-6 · Effort: high · Cost: $3.13

Superseded by updated review

[PatAKnight]

LGTM

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 12:19 PM UTC · Completed 12:32 PM UTC

Commit: fd1c54c · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.69

[fullsend-ai-retro]

Retro: PR #177 — plugin upgrade command

What happened

PR #177 added the rhdh-cli plugin upgrade command (~919 lines across 9 files). Human author gashcrumb created the PR on Sep 4. Human reviewer PatAKnight posted 3 inline comments on Sep 9. The review agent first ran on Sep 10 and completed 7 review cycles (plus 3 cancelled) across Sep 10–11, totaling $30.46 in LLM cost. The agent approved on Sep 11 (Run 7); PatAKnight approved on Sep 14; the PR merged the same day.

Review quality

The review agent performed well on correctness. Its highest-severity finding — process.exitCode = 1 being overridden by the lazy wrapper’s process.exit(0), causing install failures to silently exit with code 0 — was a genuine production bug. The agent drove 8 fix commits across 19 distinct findings (1 HIGH, 4 MEDIUM, 14 LOW), of which 14 were addressed by the author. No findings were explicitly dismissed as false positives.

Human vs. agent review delta

PatAKnight raised 2 domain-specific concerns the agent missed: (1) using Task.forCommand with targetDir instead of Task.forItem to ensure yarn install runs in the correct directory, and (2) monorepo behavior when a single backstage.json exists at the root. Both require knowledge of Backstage CLI patterns and the rhdh-cli architecture that the agent lacks due to empty AGENTS.md sections. This is the same class of gap identified in the PR #176 retro — additional evidence for redhat-developer/rhdh-cli#199. Specific content this retro suggests adding: Task.forCommand vs Task.forItem usage patterns, and the monorepo execution model for plugin commands.

Cost and iteration efficiency

The 7 completed reviews showed diminishing returns after Run 3. Runs 5–7 mostly repeated earlier findings: the syncBackstageJson error-handling concern was raised 3 times (Runs 2, 5, 6), and the compound-semver edge case was raised 3 times (Runs 4, 5, 6) while the agent itself noted it was “practically benign” each time. Three additional runs were cancelled due to rapid pushes. These patterns are already tracked:

• Review agent: persistent finding cache for cross-iteration deduplication and dismissal enforcement fullsend-ai/fullsend#2816 (finding deduplication umbrella) — syncBackstageJson finding repeated 3×
• Review agent should detect all valid findings in a single pass rather than discovering them piecemeal across iterations fullsend-ai/fullsend#4970 (single-pass detection) — 19 findings across 7 runs, piecemeal discovery
• Add configurable debounce delay before review agent dispatch on push events fullsend-ai/fullsend#7107 (dispatch debounce) — 3 cancelled runs from rapid pushes on Sep 10
• Review agent should carry forward unresolved medium+ findings across commit iterations fullsend-ai/fullsend#4956 (finding carry-forward) — README docs finding (F3) raised once then dropped without tracking

Autonomy readiness

The review agent is already providing significant value on correctness and code quality — it caught a real bug and drove iterative improvements the human reviewer did not raise. However, granting increased autonomy is premature until the domain-context gap (#199) is addressed. Once AGENTS.md includes Backstage CLI patterns and monorepo conventions, the agent would be better positioned to catch domain-specific concerns currently requiring human review.

No new proposals

All identified improvement opportunities map to existing open issues. No novel patterns were found that warrant new proposals.