Plugin system: safe deletion#259038
Conversation
ppisljar
added
release_note:skip
ppisljar
force-pushed
the
agentbuilder/plugins_3
branch
from
fa5395f to
f2accfc
Compare
ppisljar
force-pushed
the
agentbuilder/plugins_3
branch
from
f2accfc to
c41febd
Compare
…ntbuilder/plugins_3
…ntbuilder/plugins_3
| function getSkillIdsFromSource(source: AgentProperties): string[] { | ||
| return source.configuration?.skill_ids ?? source.config?.skill_ids ?? []; | ||
| } | ||
|
|
||
| function referencesSkillIds(skillIds: string[], skillIdSet: Set<string>): boolean { | ||
| return skillIds.some((id) => skillIdSet.has(id)); | ||
| } | ||
|
|
||
| function toAgentRef(source: AgentProperties, fallbackId: string): AgentRef { | ||
| const id = String(source.id ?? fallbackId); | ||
| const name = source.name; | ||
| return { id, name }; | ||
| } | ||
|
|
||
| function removeSkillIdsFromAgent(currentSkillIds: string[], skillIdsToRemove: string[]): string[] { | ||
| const removeSet = new Set(skillIdsToRemove); | ||
| return currentSkillIds.filter((id) => !removeSet.has(id)); | ||
| } |
There was a problem hiding this comment.
How does this work? When we expose a plugin to an agent, the pluginId should be in the plugin_ids field of the agent configuration, right? So we should be checking for all the agents that reference this plugin id in that field?
I'm confused that we're checking for individual skill ids in the agent.configuration 🤔 I don't think skills that are bundled as part of plugins should ever be included in the agent.configuration.skill_ids
Maybe I'm reading this all wrong, let me know 😄
There was a problem hiding this comment.
Example: none of the skill-ids for this financial-analysis plugin are included in the agent.configuration.skill_ids
There was a problem hiding this comment.
claude had the same opinion, but i was able to convince it otherwise 🥹
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Async chunks
Page load bundle
History
|
chrisbmar
left a comment
chrisbmar
left a comment
There was a problem hiding this comment.
FE changes LGTM, reviewed the ES updates too and they look good - test coverage explains it well, might want to double-check with Pierre tho if you're in doubt.
| mutationFn: ({ pluginId, force }) => pluginsService.delete({ pluginId, force }), | ||
| onSettled: () => { | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.plugins.all }); | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.skills.all }); |
There was a problem hiding this comment.
Not sure we need to do this as skills shouldn't be affected
There was a problem hiding this comment.
plugins bundle skills, so i think we need to do this after we remove them.
florent-leborgne
left a comment
florent-leborgne
left a comment
There was a problem hiding this comment.
LGTM for docs and copy
…e_for_children6 * commit '3402744f63ca1196e97b11ffac4e7f7efab240df': (80 commits) [PerUserAuth] Add EARS auth type for Connectors V2 (elastic#253695) Fix `@elastic/eui/require-aria-label-for-modals` lint violations across `@elastic/kibana-core` files (elastic#259757) [Entity Analytics][Leads generation][4] Add API routes, LeadDataClient, and async generation (elastic#257046) [Agent Builder] Agent-centric UX redesign (elastic#258005) fix query streams failing test (elastic#260277) [Lens as code] Add list layout to the new API (elastic#259967) [FTR] Add warning comments to deployment-agnostic FTR base configs (elastic#260018) [Discover][Logs profile] Fix missing search highlights (elastic#260056) Plugin system: safe deletion (elastic#259038) [Infra] Fix Hosts filter options to match selected schema (elastic#259825) Manual Entity Resolution and flyout representation (elastic#260162) [Cascade] Handle grouping on fields with unset values (elastic#260033) [Fleet] generate OTel config for integration packages with otelcol inputs (elastic#259968) [Search] Switch over to V2 index management details (elastic#259866) [inference] increase timeout for ES inference calls (elastic#260382) [ES|QL] Enable subqueries (elastic#257455) [ES|QL] Change Point order free options (elastic#260282) [Auth] Added authentication strategy for UIAM OAuth (elastic#256182) [Security Solution] Add "alerts_candidate_count" rule execution metric (elastic#259917) [api-docs] 2026-03-31 Daily api_docs build (elastic#260380) ...
5 participants
Summary
resolves https://github.com/elastic/search-team/issues/13254