Repository navigation
feat(RHIDP-18556): add spec.executionMode to agent AiResource entity - #5415
Conversation
Add an optional spawn field to the agent AiResource entity definition that declares the agent execution spawn mode. Accepted values are none (in-process), local (child process), or ephemeral (predefined sandbox). Omitting the field preserves backward compatibility — LCore applies its default. Changes: - catalog-model-ai-resource-agent: add spawn to the TypeScript interface, JSON schema (with enum constraint), and an example - catalog-backend-module-ai-resource-agent: validate spawn in collectAgentErrors and AiResourceAgentProcessor, rejecting booleans, numbers, and invalid string values - Add unit tests covering each valid value, omitted field, and invalid types across both packages - Update API report and example YAML - Add minor changesets for both packages Related to RHIDP-18556 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
PR Summary by QodoAdd optional spawn mode to agent AiResource entities
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5415 +/- ##
==========================================
+ Coverage 65.35% 65.39% +0.04%
==========================================
Files 2758 2762 +4
Lines 110579 110729 +150
Branches 31085 31107 +22
==========================================
+ Hits 72264 72407 +143
- Misses 37739 37761 +22
+ Partials 576 561 -15
*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:
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require review |
|
Important The |
|
🤖 Finished Review · ✅ Success · Started 8:26 PM UTC · Completed 8:47 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.73 |
|
Risk Assessment: moderate (2/5) DetailsBot-authored feature addition (10 files, medium blast radius, 30% test coverage) with no security, CI, or dependency changes and zero recent churn on stable paths; standard review is adequate. |
ReviewFindingsMedium
Low
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
|
/fs-fix |
|
🤖 Finished Fix · ✅ Success · Started 6:22 PM UTC · Completed 6:33 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.42 |
Add the new spec.spawn field to the README validation table, the spec.md optional-fields enumeration, and the design.md D3 field-mapping table. Fix test describe label to match the natural-language naming convention used by other describe blocks. Addresses #5415 Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔧 Fix agent — iteration 1 (human-triggered)Addressed all four review findings: added spec.spawn to the README validation table, the spec.md optional-fields list, and the design.md D3 field-mapping table; fixed the test describe label to use natural-language phrasing consistent with other describe blocks. Fixed (4):
Tests: passedNext steps:
|
gabemontero
left a comment
gabemontero
left a comment
There was a problem hiding this comment.
I'm good with the basic structure @yangcao77 ... my only quibble is the names.
Am I correct in assuming that spawn , none, local, and ephemeral are what lcore uses?
I frankly prefer you clarifications, in-process, child-process, and maybe remote for the sandbox/ephermeral choice ... end of the day we don't care if the remote env is ephemeral or permanent ... that is an implementation detail of how LCORE works currently.
And maybe instead of "spawn" the more traditional term of "execution-model" that you see for process execution an/or RPC calls
You then map those fields to whatever lcore needs
Where I am most coming from with this is that the intent @johnmcollier and I had was the eventually upstream the AiResource type agent extension. I don't think the LCORE nomenclature as currently defined would resonate upstream .... we want to be "LCORE agnostic" if you get my meaning.
WDYT?
|
/fs-fix use |
|
🤖 Finished Fix · ✅ Success · Started 8:49 PM UTC · Completed 9:04 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.36 |
…r values Rename the field from `spawn` to `executionMode` and change the enum values from 'none'|'local'|'ephemeral' to 'in-process'|'subprocess'|'sandbox' per reviewer feedback. Each value now clearly describes how the agent is executed by LCore: - `in-process` — runs inside the LCore process (no isolation) - `subprocess` — runs in a child process managed by LCore - `sandbox` — runs in an administrator-predefined sandbox profile Applied consistently across TypeScript types, JSON schema, validation logic, tests, examples, changesets, README, and OpenSpec documentation. Addresses #5415 Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix the PR title as well @yangcao77 - thanks |
🔧 Fix agent — iteration 2 (human-triggered)Renamed spec.spawn to spec.executionMode and changed enum values from none/local/ephemeral to in-process/subprocess/sandbox across all 13 affected files (TypeScript types, JSON schema, validation logic, tests, examples, changesets, README, and OpenSpec documentation). Each option is clearly documented: in-process (runs inside the LCore process, no isolation), subprocess (runs in a child process managed by LCore), sandbox (runs in an administrator-predefined sandbox profile). All 106 tests pass. Fixed (13):
Tests: passed Decision points
Next steps:
|
|
yangcao77
changed the title
gabemontero
left a comment
gabemontero
left a comment
There was a problem hiding this comment.
I'm good with where we have landed @yangcao77 thanks
I'll let you handle the remaining iterations with fullsend review and decide when to approve fullsend coder's PR for merge.
Hey, I just made a Pull Request!
Adds the optional
spec.spawnfield to the AI Catalog agent entity definition (AiResource, spec.type: agent) in the ai-integrations workspace. This field declares whether an agent runs in-process (none), in a child process (local), or in a predefined sandbox (ephemeral). When omitted, LCore applies its default.What changed
catalog-model-ai-resource-agent (minor)
spawn?: 'none' | 'local' | 'ephemeral'toAgentAiResourceEntityV1alpha1TypeScript interfacespawnenum property to the JSON schema with descriptioncatalog-backend-module-ai-resource-agent (minor)
collectAgentErrorsto validatespec.spawn— rejects booleans, numbers, and invalid string values with clear error messagesAiResourceAgentProcessorautomatically picks up the new validationTests
collectAgentErrors: accepts each valid value, missing field; rejects boolean, number, invalid string (with quoted value in message)AiResourceAgentProcessor: accepts valid spawn, accepts absent spawn, rejects invalid/boolean spawnExample
spawn: localto the technical-specialist-agent examplespawn: ephemeral✔️ Checklist
Related to https://redhat.atlassian.net/browse/RHIDP-18556
Post-script verification
agent/RHIDP-18556-add-spec-spawn)0b024a2859a0f6794c01398c5dd5f535463a965f..HEAD)