Skip to content

feat: add plugin check-versions command and RHDH version resolution engine - #176

Merged
gashcrumb merged 12 commits into
redhat-developer:mainfrom
gashcrumb:feat/plugin-check-versions
Sep 10, 2026
Merged

gashcrumb merged 12 commits into
redhat-developer:mainfrom
gashcrumb:feat/plugin-check-versions

Conversation

@gashcrumb

@gashcrumb gashcrumb commented Sep 4, 2026

Copy link
Copy Markdown
Member

Description

Implements RHIDP-16665 (RHDH-to-Backstage version mapping and manifest resolution) and RHIDP-16667 (rhdh-cli plugin check-versions) under RHIDP-13609 / RHDHPLAN-985.

Key Features

  1. Version resolution engine (src/lib/rhdhVersion.ts)

    • Resolves RHDH releases from their release-X.Y build metadata, with a static compatibility-matrix fallback when metadata is unavailable.
    • Resolves main and next from RHDH main.
    • Supports direct Backstage targets with backstage:<version>, including two-component versions such as backstage:1.54.
    • Supports minor RHDH versions and aliases: 2.0, 1.9, latest, stable, next, and main.
    • Validates URL-bound version input and remote Backstage metadata.
    • Caches manifests by version and manifest base URL.
  2. Compatibility linter (src/commands/check-versions/)

    • Adds rhdh-cli plugin check-versions.
    • Audits dependencies, devDependencies, and peerDependencies against the resolved Backstage release manifest.
    • Reports match, mismatch, unmanifested, and unverifiable dependencies. backstage:^ is reported as unverifiable because it is resolved during plugin export.
    • Supports colorized tabular output and machine-readable --json.
    • Exits with code 1 for mismatched or unmanifested dependencies.
  3. Offline and air-gapped use

    • RHDH_OFFLINE=true skips RHDH GitHub metadata lookup.
    • --manifest-file selects a local Backstage manifest.
    • Use both settings in air-gapped environments.
  4. Release and documentation

    • Bumps the CLI patch version to 2.0.5.
    • Adds a changelog entry and README usage notes.

Jira Issues

Verification

  • Focused unit tests: 35 passing
  • Type checking: yarn tsc
  • Formatting and diff checks: passing

@gashcrumb

Copy link
Copy Markdown
Member Author

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 8, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 1:00 PM UTC · Ended 1:00 PM UTC

Commit: 0408433 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 8, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure (validation failed after 2 iteration(s)) · Started 1:02 PM UTC · Completed 1:44 PM UTC

Commit: 0408433 · View workflow run →

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

@PatAKnight PatAKnight left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few inline comments and an observation.

It doesn't really seem like this repo documents any of the commands (README only
covers the janus rename and plugin package requirements). I’m not
blocking on a full guide, but I wonder if we should add a few lines in the README for
check-versions (--rhdh-version, --manifest-file, --json), especially --manifest-file for air-gapped use. This way we can at least have them started here. But if we want to keep them externally (not certain if they exist elsewhere), then that is also fine.

Comment thread src/lib/rhdhVersion.ts Outdated
Comment thread src/lib/rhdhVersion.ts Outdated
Comment thread src/commands/check-versions/command.ts Outdated
Comment thread src/lib/rhdhVersion.ts
Comment thread src/lib/rhdhVersion.ts Outdated
@kadel

kadel commented Sep 9, 2026

Copy link
Copy Markdown
Member

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure (validation failed after 1 iteration(s)) · Started 8:13 AM UTC · Completed 8:15 AM UTC

Commit: 0408433 · View workflow run →

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

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure (validation failed after 1 iteration(s)) · Started 10:17 AM UTC · Completed 10:19 AM UTC

Commit: 6e35784 · View workflow run →

Runtime: claude · Model: claude-sonnet-4-5@20250929 · Effort: high

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure (validation failed after 1 iteration(s)) · Started 10:27 AM UTC · Completed 10:29 AM UTC

Commit: c358409 · View workflow run →

Runtime: claude · Model: claude-sonnet-4-5@20250929 · Effort: high

@kadel

kadel commented Sep 9, 2026

Copy link
Copy Markdown
Member

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:04 PM UTC · Completed 12:26 PM UTC

Commit: c358409 · View workflow run →

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

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Sep 9, 2026
@fullsend-ai-review

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Moderate risk: large blast radius and substantial line count offset by no protected paths, no security-sensitive changes, good test coverage, experienced non-bot author, and mostly new files with low historical churn.

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [edge-case] src/lib/rhdhVersion.ts:250 — The explicit Backstage version path uses semver.valid() which rejects two-component versions like backstage:1.54. The input silently falls through to remote/matrix lookup and produces a misleading error: "Unsupported or unknown RHDH version". The README documents backstage:1.54.0 but users may plausibly omit the patch component.
    Remediation: Use semver.coerce(bsVer) to handle two-component versions, then extract the full version string from the coerced SemVer object.

Low

  • [logic-error] src/lib/rhdhVersion.ts:303 — The isOffline flag uses || instead of ??: options?.offline || process.env.RHDH_OFFLINE === 'true'. When a caller explicitly passes { offline: false }, the || operator treats false as falsy and falls through to the env var check, allowing RHDH_OFFLINE=true to override the explicit programmatic value. No current caller passes offline: false, but the interface type allows it.
    Remediation: Change to options?.offline ?? (process.env.RHDH_OFFLINE === 'true').

  • [scope-creep] src/commands/index.ts:182 — The process.exit(0)process.exit(process.exitCode ?? 0) change in the shared lazy() wrapper affects all commands registered through it (export, package, check-versions), not just the new feature. This also introduces a new exit-code convention (setting process.exitCode) alongside the existing throw+exitWithError pattern. Existing commands are unaffected at runtime since they never set process.exitCode.
    Remediation: Split into a separate commit/PR, or have check-versions throw a typed sentinel that lazy() recognizes.

  • [edge-case] src/commands/check-versions/command.ts:176 — The valid flag treats backstage:^ peer dependencies (status unverifiable) as invalid. Since backstage:^ is the standard version spec for peer dependencies in the Backstage ecosystem, standard plugins will always report valid: false and exit code 1, even when all resolvable dependencies match perfectly.

  • [logic-error] src/lib/rhdhVersion.ts:296 — The cache key ${normalized}:${options?.manifestFile || ''}:${options?.offline || ''} does not include versionsBaseUrl. If resolveRhdhVersion is called with the same version but different versionsBaseUrl values, the cached result from the first call is returned.

  • [test-adequacy] src/lib/rhdhVersion.test.ts — No test coverage for the getDefaultTargetVersion fallback path (invoked when rhdhVersionInput is omitted in resolveRhdhVersion). This is the default behavior when users run the command without --rhdh-version.

  • [injection] src/lib/rhdhVersion.ts:106getRhdhGitRef fallback interpolates unsanitized user input into a URL path segment (release-${version}). A crafted version string with path traversal sequences could redirect the fetch. Mitigated by: local CLI context, raw.githubusercontent.com ignores traversal, and only specific JSON fields are extracted.
    Remediation: Validate version string contains only [a-z0-9._-] before URL interpolation.

  • [injection] src/lib/rhdhVersion.ts:149backstageVersion from remote build-metadata.json passes through semver.clean() || bsVersion.trim(), allowing a malformed string to flow to getManifestByVersion(). Mitigated by: remote data comes from a trusted Red Hat repository.
    Remediation: Validate validBsVersion is well-formed semver before returning.

  • [import-organization] src/commands/check-versions/command.ts:17 — All imports in a single unseparated block. Sibling command files (e.g., export-dynamic-plugin/command.ts) separate imports into groups with blank lines.
    Remediation: Add blank lines between import groups.

  • [import-organization] src/lib/rhdhVersion.ts:17 — No blank line between third-party import (semver) and relative import. Sibling backstageVersion.ts uses blank lines between groups.
    Remediation: Add blank line between semver and relative imports.

  • [import-organization] src/lib/rhdhVersion.test.ts:17 — Relative import (clearManifestCache) placed before third-party imports (fs-extra, os). Codebase convention places relative imports after third-party.
    Remediation: Reorder so third-party imports come first.

  • [import-consistency] src/lib/rhdhVersion.test.ts:19 — Mixed node: protocol prefix: import os from "os" alongside import path from "node:path". Same in command.test.ts.

  • [naming-coherence] src/commands/check-versions/command.ts — The versions:lint alias mirrors the directory path of the existing src/commands/versions/lint.ts utility module, which could mislead contributors into thinking they are related.

  • [api-shape] src/commands/check-versions/command.ts:62CheckVersionsOptions includes a json property that is a CLI presentation concern. checkPluginDependencies() never reads it — only command() uses it for output formatting.


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

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread src/lib/rhdhVersion.ts Outdated
Comment thread src/lib/rhdhVersion.ts Outdated
Comment thread src/commands/index.ts Outdated
Comment thread src/commands/check-versions/command.ts Outdated
Comment thread src/lib/rhdhVersion.ts Outdated
Comment thread src/lib/rhdhVersion.ts
* limitations under the License.
*/

import semver from 'semver';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] import-organization

No blank line between third-party import (semver) and relative import. Sibling backstageVersion.ts uses blank lines between groups.

Suggested fix: Add blank line between semver and relative imports.

Comment thread src/lib/rhdhVersion.test.ts Outdated
Comment thread src/lib/rhdhVersion.test.ts Outdated
@@ -0,0 +1,292 @@
/*

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] naming-coherence

The versions:lint alias mirrors the directory path of existing src/commands/versions/lint.ts utility module, potentially misleading contributors.

Comment thread src/commands/check-versions/command.ts Outdated
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure (validation failed after 1 iteration(s)) · Started 12:53 PM UTC · Completed 12:55 PM UTC

Commit: b9deaf8 · View workflow run →

Runtime: claude · Model: claude-sonnet-4-5@20250929 · Effort: high

…ngine (RHIDP-16665, RHIDP-16667)

Assisted-by: opencode

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
- Use node:path instead of path
- Reduce cognitive complexity in checkPluginDependencies and resolveRhdhVersion
- Remove nested template literals in linter output
- Use optional chaining for cached manifest
- Deduplicate test setup fixtures in check-versions unit tests

Assisted-by: opencode

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Assisted-by: opencode

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Assisted-by: opencode

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
…re blocks

Assisted-by: opencode

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
- Fall back to main branch when targeting in-development releases before branch cuts
- Support direct Backstage version specifiers (e.g. backstage:1.54.0 or 1.54.0)
- Update 2.1.0 and main compatibility matrix to Backstage 1.54.0

Assisted-by: opencode

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Assisted-by: opencode

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Assisted-by: openai/gpt-5.6-terra

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Assisted-by: openai/gpt-5.6-terra

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@gashcrumb
gashcrumb force-pushed the feat/plugin-check-versions branch from b9deaf8 to e9c5e7e Compare September 9, 2026 12:57
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 9, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 12:58 PM UTC · Ended 1:00 PM UTC

Commit: e9c5e7e · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure (validation failed after 1 iteration(s)) · Started 12:58 PM UTC · Completed 1:00 PM UTC

Commit: e9c5e7e · View workflow run →

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

Assisted-by: openai/gpt-5.6-terra

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure (validation failed after 1 iteration(s)) · Started 1:16 PM UTC · Completed 1:17 PM UTC

Commit: 3b1a42e · View workflow run →

Runtime: claude · Model: opus → claude-opus-5 · Effort: high

Assisted-by: openai/gpt-5.6-terra

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure (validation failed after 1 iteration(s)) · Started 1:20 PM UTC · Completed 1:22 PM UTC

Commit: bd68345 · View workflow run →

Runtime: claude · Model: opus → claude-opus-5 · Effort: high

Assisted-by: openai/gpt-5.6-terra

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure (validation failed after 1 iteration(s)) · Started 3:48 PM UTC · Completed 3:49 PM UTC

Commit: 687923a · View workflow run →

Runtime: claude · Model: opus → claude-opus-5 · Effort: high

@PatAKnight PatAKnight left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks pretty good to me, just one final question.

Comment thread CHANGELOG.md

@PatAKnight PatAKnight left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gashcrumb
gashcrumb merged commit 72ee02c into redhat-developer:main Sep 10, 2026
55 of 56 checks passed
@fullsend-ai-retro

fullsend-ai-retro Bot commented Sep 10, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 12:31 PM UTC · Completed 12:50 PM UTC

Commit: 687923a · View workflow run →

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

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #176 – plugin check-versions command

Timeline

  1. Sep 4 – PR opened by gashcrumb: new plugin check-versions command with RHDH version resolution engine (+1373/−11 across 10 files)
  2. Sep 8 – First /fs-review triggered → review agent failed (opus/claude-opus-4-6, validation failure — agent-result.json not found)
  3. Sep 9 01:00–01:38 – Human reviewer PatAKnight posts 5 substantive inline comments on design-level concerns (fallback behavior, air-gap semantics, version input ambiguity, static matrix maintenance, unprefixed version UX)
  4. Sep 9 08:13 – Second /fs-review by kadel → failed (claude-sonnet-4-6 not available on Vertex)
  5. Sep 9 10:17–10:27 – Two auto-triggered runs failed (claude-sonnet-4-5@20250929 not available on Vertex)
  6. Sep 9 10:28–11:17 – Author responds to human review and pushes fixes
  7. Sep 9 12:02 – Third /fs-review by kadel → succeeded (opus/claude-opus-4-6, $8.81, 14 findings posted as CHANGES_REQUESTED)
  8. Sep 9 12:51–15:48 – Five more auto-triggered runs all failed (1× model not available, 4× IAM permission denied on claude-opus-5)
  9. Sep 10 11:53 – PatAKnight approves; PR merged at 12:30

Review agent reliability: 1/10 success rate

Of 10 review agent runs, 9 failed before producing any output:

  • 4 failures: claude-sonnet-4-5@20250929 / claude-sonnet-4-6 not available on Vertex deployment
  • 4 failures: IAM aiplatform.endpoints.predict permission denied on claude-opus-4-6 / claude-opus-5
  • 1 failure: Agent execution produced no agent-result.json (validation failure after 2 iterations)

The model alias opus resolved to claude-opus-4-6 in earlier runs but to claude-opus-5 in later runs, suggesting an upstream alias change mid-PR compounded the IAM failures.

This evidence corroborates several existing open issues (no new proposals filed):

  • fullsend#7026 – no automatic fallback when aliased model is unavailable; 4 sonnet failures occurred before users manually retried with opus
  • fullsend#6967 – no fallback on policy_denied; 4 IAM failures wasted runs
  • fullsend#1032 – pre-flight Vertex AI authorization check would have caught IAM issues immediately
  • fullsend#7104 – no auto re-dispatch on infrastructure error; all retries required manual /fs-review
  • fullsend#7023 – status comments collapsed model-unavailability, IAM-denied, and agent-execution failures into one generic "validation failed" message

Review quality: agent vs. human

The human reviewer caught 5 design-level concerns the agent missed entirely: fallback-to-main giving wrong Backstage version, RHDH_OFFLINE not actually enabling air-gapped use, backstage:^ silently treated as unverifiable, static matrix maintenance burden, and unprefixed version input ambiguity. These required domain knowledge of RHDH release engineering and user workflows.

The agent's one medium finding (semver.valid() rejecting two-component versions like backstage:1.54) was valid and actionable. However, 3 of 14 findings (21%) were import-organization noise (import sorting, blank lines between groups, mixed node: prefix) despite the repo having eslint-plugin-import configured. This corroborates agents#493 – the style sub-agent should check for linter/formatter config before flagging style issues that automated tooling already covers.

Autonomy assessment

This PR does not support increased agent review autonomy. The agent missed all domain-level design concerns, and the 9/10 infrastructure failure rate meant the single successful review arrived after the human had already provided the most impactful feedback. The repo's AGENTS.md has empty Architecture, Key Conventions, and Pattern References sections — see proposal below.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants