feat(#4129): add AiResourceAgentProcessor for agent field validation - #4246
Conversation
…essor Extend AIResourceExtensionsProcessor to validate agent-specific fields when spec.type is 'agent'. Add collectAgentErrors helper (following the collectOciErrors pattern) that checks: - spec.instructions: required, non-empty string - spec.handoffs / spec.tools: must be arrays if present (opaque strings accepted without entity-ref format enforcement) - spec.resetToolChoice: must be boolean if present - spec.modelSettings: must be plain object if present - spec.toolUseBehavior: must be string or string array if present - spec.outputSchema: must be string or object if present - spec.handoffDescription / spec.model: must be strings if present All errors are collected and reported together. Non-agent AiResource entities (skill, rule, model) are unaffected. Error messages name the field path and problem without exposing internal class names. Closes #4129
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4246 +/- ##
==========================================
+ Coverage 59.59% 59.61% +0.02%
==========================================
Files 2458 2460 +2
Lines 98268 98330 +62
Branches 27444 27475 +31
==========================================
+ Hits 58559 58620 +61
- Misses 38074 38075 +1
Partials 1635 1635
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
johnmcollier
left a comment
johnmcollier
left a comment
There was a problem hiding this comment.
Request changes
Agent field validation landed in the wrong place relative to #4128 / #4164. Please rework this PR rather than polishing the current AIResourceExtensionsProcessor approach.
1) Follow the previous PR’s packaging approach
#4128 put agent typing in dedicated packages:
catalog-model-ai-resource-agent(types / JSON schema /KindValidator)catalog-backend-module-ai-resource-agent(catalog model registration forkind: AiResource)
Do not extend AIResourceExtensionsProcessor with agent-specific field rules. That processor should stay focused on shared RHDH extensions (spec.scope, OCI source-location).
If RHIDP-15868 still needs catalog-processor coverage beyond the existing agent KindValidator, put it in the agent backend module (matching AiResource), not in catalog-backend-module-ai-resource-extensions.
Also align kind casing with #4128 (AiResource), not AIResource.
2) Update OpenSpec to match that architecture
The current OpenSpec steered this PR incorrectly (notably D10 / tasks §5 / proposal “extend the AiResource extensions processor path”). Update the OpenSpec change materials under workspaces/ai-integrations/openspec/changes/airesource-agent-typed-schema/ so they reflect:
- Agent schema + agent ingestion validation live with the agent packages / module
AIResourceExtensionsProcessorremains scope/OCI only- Kind spelling
AiResourceconsistent with examples and the agent model
Please amend design.md, tasks.md, proposal.md, and the ai-resource-agent-ingestion spec as needed so the next coding pass cannot re-learn the wrong home.
Out of scope for this fix round
Fullsend workflow/skill changes for OpenSpec drift feedback loops — track separately.
|
/fs-fix |
|
🤖 Finished Fix · ✅ Success · Started 8:28 PM UTC · Completed 8:46 PM UTC Commit: |
Move collectAgentErrors and agent field validation out of AIResourceExtensionsProcessor into catalog-backend-module-ai-resource-agent as AiResourceAgentProcessor, matching the packaging approach from #4128. - Remove agent-specific code from AIResourceExtensionsProcessor (scope/OCI only) - Add AiResourceAgentProcessor to catalog-backend-module-ai-resource-agent - Register processor alongside model source in the agent backend module - Update OpenSpec design.md, tasks.md, proposal.md to reflect architecture - Align kind casing with AiResource (matching #4128) Addresses review feedback on #4246 Assisted-by: Claude Code
🔧 Fix agent — iteration 1 (human-triggered)Addressed all 4 review findings. Moved agent field validation from AIResourceExtensionsProcessor to a new AiResourceAgentProcessor in catalog-backend-module-ai-resource-agent. Updated OpenSpec documents (design.md, tasks.md, proposal.md) to reflect the correct architecture. All tests pass (102 tests across both packages). Fixed (4):
Tests: passed Decision points
Updated by fullsend fix agent |
|
/fs-review |
1 similar comment
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 9:53 PM UTC · Completed 10:10 PM UTC Commit: |
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
|
fullsend-ai-review
Bot
added
the
requires-manual-review
johnmcollier
left a comment
johnmcollier
left a comment
There was a problem hiding this comment.
Second review pass
Architecture move looks right: agent field validation now lives in catalog-backend-module-ai-resource-agent as AiResourceAgentProcessor (alongside the agent model), and AIResourceExtensionsProcessor stays scope/OCI-only. That packaging split is intentional — not a problem to reverse.
Please address the remaining actionable items from the bot review (and stale PR metadata):
1) Update PR title
Current title still says “add agent field validation to AIResource extensions processor.” Update to something accurate, e.g.:
feat(#4129): add AiResourceAgentProcessor for agent field validation
2) Add package README
Add workspaces/ai-integrations/plugins/catalog-backend-module-ai-resource-agent/README.md documenting:
- what the module registers (agent catalog model +
AiResourceAgentProcessor) - that it targets
kind: AiResource/spec.type: agent - which agent fields are validated (mirror the processor JSDoc / sibling extensions README style)
- that shared RHDH extensions (scope / OCI) remain in
catalog-backend-module-ai-resource-extensions
3) Refresh PR description
Replace the body that still describes extending AIResourceExtensionsProcessor. Briefly note:
- validation lives in the agent backend module (same packaging approach as #4128)
- OpenSpec (
airesource-agent-typed-schema) was updated to match - extensions processor is unchanged for agent fields
Do not change
- Whitespace-only
instructions/ array element-type checks — leave as intentional schema/processor layering - Do not move validation back into
AIResourceExtensionsProcessor
Addressing title/README/PR body directly instead of routing through fullsend.
johnmcollier
changed the title
Add package README for the agent backend module and a minor changeset for the new ingestion processor. PR title/description updated separately. Assisted-by: Cursor Grok 4.5 Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve module.ts conflict by keeping main's CatalogModelSource registration pattern and AiResourceAgentProcessor registration. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 7:13 PM UTC · Completed 7:29 PM UTC Commit: |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the review comment for full details.
Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:
workspaces/ai-integrations/plugins/catalog-backend-module-ai-resource-agent/src/AiResourceAgentProcessor.ts:53: [medium] naming-coherence
The new processor checks entity.kind !== 'AiResource' while AIResourceExtensionsProcessor checks entity.kind !== 'AIResource'. The two processors within the same workspace fire on different kind spellings, meaning scope/OCI validation from the extensions processor will not apply to agent entities using 'AiResource'. This is not necessarily a bug if the kinds are genuinely registered separately, but deserves explicit documentation.
workspaces/ai-integrations/plugins/catalog-backend-module-ai-resource-agent/src/collectAgentErrors.ts:35: [low] edge-case
spec.instructions validation accepts whitespace-only strings because it checks === '' without trimming. The design doc (D3) says 'Non-empty string only' and a whitespace-only instruction is functionally empty.
Suggested fix: Consider adding .trim() before the empty-string check.
workspaces/ai-integrations/plugins/catalog-backend-module-ai-resource-agent/src/module.ts(file-level): Line 28 · [low] stale-doc
JSDoc on catalogModuleAiResourceAgent says 'Registers the agent specType for the AiResource kind in the catalog' but the module now also registers AiResourceAgentProcessor for agent field validation. The JSDoc does not mention the processor registration.
Suggested fix: Update JSDoc to mention both responsibilities.
AIResource is not a valid catalog kind. Align extensions processor, public exports, README, OpenSpec, and examples on AiResource only. Assisted-by: Cursor Grok 4.5 Co-authored-by: Cursor <cursoragent@cursor.com>
gabemontero
left a comment
gabemontero
left a comment
There was a problem hiding this comment.
Aside from the in-line comment I have @johnmcollier here is a copy/paste of the report the openspec audit skill compiled for me when pointing it at your PR:
● Audit Report: airesource-agent-typed-schema
Last audited: 2026-08-13T16:05:00Z
Summary
┌──────────────────────────────────────┬──────────┬─────────┬────────────┐
│ Category │ CRITICAL │ WARNING │ SUGGESTION │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ A Entity propagation │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ B Enum / vocabulary │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ C Semantic contradiction │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ D Codebase & convention grounding │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ E Namespace & cross-change ownership │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ F Template / copy-paste residue │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ G Extended coherence │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ H Security lint │ 0 │ 0 │ 0 │
└──────────────────────────────────────┴──────────┴─────────┴────────────┘
CRITICAL
- None
WARNING
- None
SUGGESTION
- None
---
Audit Report: ai-resource-catalog-entity-kind
Last audited: 2026-08-13T16:05:00Z
Summary
┌──────────────────────────────────────┬──────────┬─────────┬────────────┐
│ Category │ CRITICAL │ WARNING │ SUGGESTION │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ A Entity propagation │ 0 │ 1 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ B Enum / vocabulary │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ C Semantic contradiction │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ D Codebase & convention grounding │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ E Namespace & cross-change ownership │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ F Template / copy-paste residue │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ G Extended coherence │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ H Security lint │ 0 │ 0 │ 0 │
└──────────────────────────────────────┴──────────┴─────────┴────────────┘
CRITICAL
- None
┌──────────────────────────────────────┬──────────┬─────────┬────────────┐
│ Category │ CRITICAL │ WARNING │ SUGGESTION │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ A Entity propagation │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ B Enum / vocabulary │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ C Semantic contradiction │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ D Codebase & convention grounding │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ E Namespace & cross-change ownership │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ F Template / copy-paste residue │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ G Extended coherence │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ H Security lint │ 0 │ 0 │ 0 │
└──────────────────────────────────────┴──────────┴─────────┴────────────┘
CRITICAL
- None
WARNING
- None
SUGGESTION
- None
---
Audit Report: ai-resource-catalog-entity-kind
Last audited: 2026-08-13T16:05:00Z
Summary
┌──────────────────────────────────────┬──────────┬─────────┬────────────┐
│ Category │ CRITICAL │ WARNING │ SUGGESTION │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ A Entity propagation │ 0 │ 1 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ B Enum / vocabulary │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ C Semantic contradiction │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ D Codebase & convention grounding │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ E Namespace & cross-change ownership │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ F Template / copy-paste residue │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ G Extended coherence │ 0 │ 0 │ 0 │
├──────────────────────────────────────┼──────────┼─────────┼────────────┤
│ H Security lint │ 0 │ 0 │ 0 │
└──────────────────────────────────────┴──────────┴─────────┴────────────┘
CRITICAL
- None
WARNING
- (A) workspaces/ai-integrations/openspec/changes/ai-resource-catalog-entity-kind/tasks.md — Tasks 2.1–2.4 reference AiResourceOciProcessor (4 occurrences), but the implementation and sibling change airesource-agent-typed-schema D5 use AiResourceExtensionsProcessor as the canonical processor name for scope/OCI validation. Open tasks 2.2 and 2.4 direct implementers to a class that no longer exists under that name. Recommendation: Rename AiResourceOciProcessor
→ AiResourceExtensionsProcessor in tasks 2.1–2.4, or add a parenthetical noting the rename. (autofixable)
SUGGESTION
- None
---
Audit clean (no CRITICAL) — the one WARNING is a mechanical name drift in tasks.md that's autofixable. airesource-agent-typed-schema is fully consistent across all artifacts.
Agents may bake a default system prompt into the image/runtime, so catalog authors should not be forced to invent a placeholder. Keep type-checking when the field is present. Assisted-by: Cursor Grok 4.5 Co-authored-by: Cursor <cursoragent@cursor.com>
|
In case it got lost in the noise @johnmcollier from my earlier copy/past here was the sole bit from the openspec audit review: WARNING
WDYT? |
…ssor Rename stale AiResourceOciProcessor references in the catalog-entity-kind OpenSpec tasks and mark the landed source-location retool work complete. Assisted-by: Cursor Grok 4.5 Co-authored-by: Cursor <cursoragent@cursor.com>
|
D'oh missed that. Looks good, updated. Really nice to see it in action 👍 |
|
|
🤖 Finished Retro · ✅ Success · Started 11:29 PM UTC · Completed 11:45 PM UTC Commit: |
Retro: PR #4246 — AiResourceAgentProcessor for agent field validationWorkflow overviewPR #4246 was a bot-authored PR (by Timeline
What went well
Evidence for existing tracked issues
No new proposalsAll improvement opportunities from this workflow are already tracked in the issues above. The most impactful improvement would be landing #4232, which would prevent the recurring architecture mismatch in the ai-integrations workspace. |
2 participants
Add
AiResourceAgentProcessorincatalog-backend-module-ai-resource-agentto validate agent-specific fields whenspec.typeisagent, following the same packaging approach as #4128 / #4164 (dedicated agent model + backend module forkind: AiResource).What changed
AiResourceAgentProcessoralongside the agent catalog model sourcespec.instructionsand optional agent field shapes (handoffs,tools,resetToolChoice,modelSettings,toolUseBehavior,outputSchema,handoffDescription,model)handoffs/toolsstring arrays without entity-ref format enforcementAIResourceExtensionsProcessorfocused on shared RHDH extensions (spec.scope, OCIsource-location) — no agent field rules thereairesource-agent-typed-schema) so design/tasks/proposal match this architecturePackaging note
Agent typing stays in the agent packages rather than a single shared “AI experience” extensions processor. That split is intentional.
Closes #4129