Skip to content

feat(#3694): add AIResource spec.scope validation#3742

Open
fullsend-ai-coder[bot] wants to merge 4 commits into
mainfrom
agent/3694-airesource-spec-scope
Open

feat(#3694): add AIResource spec.scope validation#3742
fullsend-ai-coder[bot] wants to merge 4 commits into
mainfrom
agent/3694-airesource-spec-scope

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Add RHDH-specific spec.scope extension support for AIResource entities. The AIResourceScopeValidator CatalogProcessor validates that spec.scope, when present, is one of 'organization', 'product', or 'team'. Omitting spec.scope is valid. Invalid values produce actionable errors naming the field, received value, and accepted values.

Changes:

  • AIResourceScopeValidator processor in preProcessEntity validates
    spec.scope on AIResource entities only
  • module.ts registers the processor via catalogProcessingExtensionPoint
  • index.ts exports the validator, type, and constants for reuse
  • 16 tests covering valid, invalid, omitted scope, error quality,
    and non-AIResource pass-through

Assisted-by: Claude Opus 4.6


Closes #3694

Post-script verification

  • Branch is not main/master (agent/3694-airesource-spec-scope)
  • Secret scan passed (gitleaks — 3e8f96f5479023ed9fcb89b5f373ba8d26b42474..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Add RHDH-specific spec.scope extension support for AIResource entities.
The AIResourceScopeValidator CatalogProcessor validates that spec.scope,
when present, is one of 'organization', 'product', or 'team'. Omitting
spec.scope is valid. Invalid values produce actionable errors naming
the field, received value, and accepted values.

Changes:
- AIResourceScopeValidator processor in preProcessEntity validates
  spec.scope on AIResource entities only
- module.ts registers the processor via catalogProcessingExtensionPoint
- index.ts exports the validator, type, and constants for reuse
- 16 tests covering valid, invalid, omitted scope, error quality,
  and non-AIResource pass-through

Closes #3694

Assisted-by: Claude Opus 4.6
@rhdh-gh-app

rhdh-gh-app Bot commented Jul 13, 2026

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-catalog-backend-module-catalog-backend-module-ai-resource-extensions workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions minor v0.1.0

@johnmcollier

Copy link
Copy Markdown
Member

/fs-fix Fix the failing CI checks

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 8:26 PM UTC · Completed 8:41 PM UTC
Commit: 3e8f96f · View workflow run →

Change makeAIResource spec parameter type from Record<string, unknown>
to Entity['spec'] to match the Entity type definition, fixing TS2322
type-check CI failure.

Addresses review feedback on #3742

Assisted-by: Claude Opus 4.6
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Fixed failing CI type-check (TS2322) by correcting the spec parameter type in the test helper function. The change is a single-line type annotation fix with no behavioral impact.

Fixed (1):

  1. TS2322 type error in AIResourceScopeValidator.test.ts (workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/AIResourceScopeValidator.test.ts): Changed makeAIResource spec parameter type from Record<string, unknown> to Entity['spec'] to match the Entity type definition. Record<string, unknown> is not assignable to JsonObject because unknown is not assignable to JsonValue. Using Entity['spec'] derives the exact type from the already-imported Entity, avoiding both the type error and the need for a new @backstage/types dependency.

Tests: passed

Updated by fullsend fix agent

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.60%. Comparing base (3e8f96f) to head (ddb97b6).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3742   +/-   ##
=======================================
  Coverage   54.59%   54.60%           
=======================================
  Files        2360     2362    +2     
  Lines       90121    90145   +24     
  Branches    25204    25207    +3     
=======================================
+ Hits        49204    49223   +19     
- Misses      40672    40687   +15     
+ Partials      245      235   -10     
Flag Coverage Δ *Carryforward flag
adoption-insights 83.70% <ø> (ø) Carriedforward from dd16032
ai-integrations 67.92% <90.47%> (+0.39%) ⬆️
app-defaults 69.79% <ø> (ø) Carriedforward from dd16032
augment 46.39% <ø> (ø) Carriedforward from dd16032
boost 73.07% <ø> (ø) Carriedforward from dd16032
bulk-import 72.46% <ø> (ø) Carriedforward from dd16032
cost-management 14.10% <ø> (ø) Carriedforward from dd16032
dcm 61.81% <ø> (ø) Carriedforward from dd16032
extensions 61.53% <ø> (ø) Carriedforward from dd16032
global-floating-action-button 71.18% <ø> (ø) Carriedforward from dd16032
global-header 59.71% <ø> (ø) Carriedforward from dd16032
homepage 50.30% <ø> (ø) Carriedforward from dd16032
install-dynamic-plugins 56.77% <ø> (ø) Carriedforward from dd16032
konflux 91.49% <ø> (ø) Carriedforward from dd16032
lightspeed 69.02% <ø> (ø) Carriedforward from dd16032
mcp-integrations 85.46% <ø> (ø) Carriedforward from dd16032
orchestrator 42.97% <ø> (ø) Carriedforward from dd16032
quickstart 65.63% <ø> (ø) Carriedforward from dd16032
sandbox 79.56% <ø> (ø) Carriedforward from dd16032
scorecard 82.93% <ø> (ø) Carriedforward from dd16032
theme 61.26% <ø> (ø) Carriedforward from dd16032
translations 7.25% <ø> (ø) Carriedforward from dd16032
x2a 78.68% <ø> (ø) Carriedforward from dd16032

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e8f96f...ddb97b6. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@johnmcollier

Copy link
Copy Markdown
Member

/fs-fix Fix the failing API reports error on the CI check

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 8:53 PM UTC · Completed 9:06 PM UTC
Commit: 3e8f96f · View workflow run →

Regenerate report.api.md to include AIResourceScopeValidator,
VALID_AI_RESOURCE_SCOPES, and AIResourceScope type exports that were
added in the PR but missing from the API report, causing CI to fail.

Addresses review feedback on #3742

Assisted-by: Claude Opus 4.6
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 2 (human-triggered)

Regenerated the API report file to reflect the new public exports added by the PR. The report.api.md was outdated and only contained the default module export, while index.ts now also exports AIResourceScopeValidator, VALID_AI_RESOURCE_SCOPES, and the AIResourceScope type. Running yarn build:api-reports updated the report, and re-running it confirms no further changes are needed. All 16 tests pass.

Fixed (1):

  1. Failing API reports CI check (workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/report.api.md): Regenerated report.api.md to include the new public exports (AIResourceScopeValidator class, VALID_AI_RESOURCE_SCOPES constant, and AIResourceScope type) that were added in the PR but missing from the API report, causing the CI api-reports check to fail.

Tests: passed

Updated by fullsend fix agent

@johnmcollier

Copy link
Copy Markdown
Member

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 13, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:36 PM UTC · Completed 9:50 PM UTC
Commit: 3e8f96f · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review

Verdict: comment · 2 medium, 6 low findings

The PR correctly implements AIResource spec.scope validation as a CatalogProcessor, matching the requirements in issue #3694 and the openspec at ai-resource-entity-schema/spec.md. The validator logic is correct, the module registration follows the standard Backstage backend module pattern, tests cover the core scenarios, and the API surface is properly exported. The behavioral change from a no-op "Hello World!" scaffold to actual validation is intentional for this pre-1.0 (0.1.0) package with no external consumers.

No critical or high severity issues were found. The medium-severity findings are about naming convention consistency and documentation — they are worth addressing but do not block the PR.


Medium

1. Naming convention: AIResourceScopeValidator vs *Processor suffix

File: workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/AIResourceScopeValidator.ts
Category: naming-convention

All six existing CatalogProcessor implementations in this repo use the Processor suffix (e.g., ExtensionsPluginProcessor, ExtensionsCollectionProcessor, LocalPackageInstallStatusProcessor). The two Validator-named classes in the repo (McpServerValidator, RequestValidator) do not implement CatalogProcessor. Since AIResourceScopeValidator implements CatalogProcessor, the naming departs from the established convention.

Remediation: Consider renaming to AIResourceScopeProcessor for consistency with the repo's naming convention for CatalogProcessor implementations. This would also affect the test file, exports in index.ts, and report.api.md.

2. README remains scaffold placeholder

File: workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/README.md
Category: stale-readme

The README still contains only the Backstage CLI scaffold text ("This plugin was created through the Backstage CLI"). With the PR adding real validation functionality, the README should document what the module does, the valid spec.scope values, and the public API surface.

Remediation: Update the README to describe the module's purpose (AIResource entity spec.scope validation), valid scope values (organization, product, team), and the exported symbols.


Low

3. Missing test for scope: null

File: workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/AIResourceScopeValidator.test.ts
Category: edge-case

In YAML, scope: null or scope: ~ produces a null value at runtime. The production code handles this correctly (null !== undefined is true, so null enters the validation branch and is rejected), but no test covers this case. A future refactor changing the !== undefined guard to a falsy check could silently allow null through.

Remediation: Add a test: it('should reject null scope value', ...).

4. Missing test for entity with no spec property

File: workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/AIResourceScopeValidator.test.ts
Category: test-adequacy

The makeAIResource helper always sets spec, but an AIResource entity could arrive with spec entirely absent. The optional chaining (entity.spec?.scope) handles this correctly, but no test covers it.

Remediation: Add a test with an AIResource entity that has no spec key.

5. Error message reflects unsanitized user input

File: workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/AIResourceScopeValidator.ts
Category: information-disclosure

The error message includes the user-controlled scope value via String(scope) without truncation or control-character stripping. While the practical risk is low (Backstage catalog errors are typically shown to authenticated admins), an adversarial value could contain ANSI escapes or be arbitrarily long.

Remediation: Consider truncating and sanitizing the reflected value: String(scope).replace(/[\x00-\x1f]/g, '').slice(0, 200).

6. Test file location

File: workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/AIResourceScopeValidator.test.ts
Category: code-organization

Sibling plugins with CatalogProcessor implementations place processor files in a processors/ subdirectory. For a single-processor plugin this is less critical, but worth considering for future consistency as more processors are added (e.g., the OCI validator from task 1.4).

7. JSDoc tag on index.ts

File: workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/index.ts
Category: documentation-comment-format

The entry point uses @public as its JSDoc tag, while sibling modules (catalog-backend-module-model-catalog, ai-experience-common) use @packageDocumentation, which is more appropriate for a module entry point.

8. Changeset describes scaffolding, not the feature

File: workspaces/ai-integrations/.changeset/short-insects-cut.md
Category: stale-changeset

The existing changeset text ("Scaffold new AIResource catalog extension") describes the prior scaffolding PR. This PR should carry its own changeset describing the actual feature: AIResource spec.scope validation.

Remediation: Run yarn changeset to create a new changeset with a minor bump describing the validation feature.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jul 13, 2026
@johnmcollier

Copy link
Copy Markdown
Member

/fs-fix address review feedback

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 1:15 AM UTC · Completed 1:33 AM UTC
Commit: 3e8f96f · View workflow run →

- Rename AIResourceScopeValidator to AIResourceScopeProcessor to match
  repo naming convention for CatalogProcessor implementations (finding 1)
- Update README from scaffold placeholder to document module purpose,
  valid scope values, and public API surface (finding 2)
- Add test for scope: null edge case (finding 3)
- Add test for entity with no spec property (finding 4)
- Sanitize error message: truncate to 200 chars and strip control
  characters from reflected user input (finding 5)
- Update index.ts JSDoc tag from @public to @packageDocumentation to
  match sibling module convention (finding 7)
- Add new changeset describing the validation feature (finding 8)
- Regenerate API report for renamed class

Addresses review feedback on #3742

Assisted-by: Claude Opus 4.6
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 3 (human-triggered)

Addressed 7 of 8 review findings. Disagreed with 1 (test file location in processors/ subdirectory - deferred until a second processor is added). Key changes: renamed AIResourceScopeValidator to AIResourceScopeProcessor for convention consistency, added 2 edge-case tests, sanitized error message input, updated README and JSDoc, created feature changeset.

Fixed (7):

  1. *Naming convention: AIResourceScopeValidator vs Processor suffix (workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/AIResourceScopeProcessor.ts): Renamed AIResourceScopeValidator to AIResourceScopeProcessor to match repo naming convention for CatalogProcessor implementations. Updated class name, file names, exports, imports, module registration, and regenerated API report.
  2. README remains scaffold placeholder (workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/README.md): Replaced scaffold placeholder with documentation describing module purpose (AIResource spec.scope validation), valid scope values, and the public API surface.
  3. Missing test for scope: null (workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/AIResourceScopeProcessor.test.ts): Added test 'should reject null scope value' covering the YAML scope: null / scope: ~ edge case.
  4. Missing test for entity with no spec property (workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/AIResourceScopeProcessor.test.ts): Added test 'should accept entity with no spec property' covering AIResource entities arriving without a spec key.
  5. Error message reflects unsanitized user input (workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/AIResourceScopeProcessor.ts): Added input sanitization: control characters (charCode <= 0x1f) are stripped and value is truncated to 200 characters before inclusion in error message. Used Array.from().filter() approach to avoid triggering ESLint no-control-regex rule.
  6. JSDoc tag on index.ts should use @packageDocumentation (workspaces/ai-integrations/plugins/catalog-backend-module-catalog-backend-module-ai-resource-extensions/src/index.ts): Changed index.ts JSDoc tag from @public to @packageDocumentation to match sibling modules (catalog-backend-module-model-catalog, ai-experience-common). Also updated the description text to match the convention.
  7. Changeset describes scaffolding, not the feature (workspaces/ai-integrations/.changeset/ai-resource-scope-validation.md): Created new changeset 'ai-resource-scope-validation.md' with minor bump describing the actual feature: AIResource spec.scope validation via AIResourceScopeProcessor.

Disagreed (1):

  1. Test file location in processors/ subdirectory: The reviewer noted this is 'less critical' for a single-processor plugin. The rename to *Processor (finding 1) is the higher-value consistency fix. Moving to a processors/ subdirectory adds churn without proportional benefit for a single file, and would be better addressed when a second processor is added (e.g., the OCI validator from task 1.4).

Tests: passed

Decision points
  • Chose Array.from().filter() sanitization over regex to avoid ESLint no-control-regex rule violation (alternatives: Regex with eslint-disable comment, new RegExp() constructor; rationale: The eslint-disable comment was not suppressing the error in the lint-staged hook. Array.from().filter() achieves the same sanitization without triggering the rule.)

Updated by fullsend fix agent

@sonarqubecloud

Copy link
Copy Markdown

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

Labels

ready-for-review Agent PR ready for human review requires-manual-review Review requires human judgment workspace/ai-integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ai-integrations: add AIResource spec.scope schema extension

1 participant