[One Workflow] Step registry completion context enhancement#260054
[One Workflow] Step registry completion context enhancement#260054semd merged 18 commits intoelastic:mainfrom
Conversation
…stry_completion_improvement' into workflows/step_registry_completion_improvement
…stry_completion_improvement' into workflows/step_registry_completion_improvement
…stry_completion_improvement' into workflows/step_registry_completion_improvement
| @@ -29,10 +31,16 @@ | |||
There was a problem hiding this comment.
This was never true, actually 😅
These changes don't make this type wrong, they just make getValueFromValueNode handle the reality. The type annotation is a pre-existing inaccuracy.
I updated the comment to note the special case here: 49520cc
The suggestion from Cursor:
That's ~37 usages across 12 files. Many of them access .valueNode.range, .valueNode.value, or pass it to APIs expecting Scalar. Widening the type to Scalar | YAMLSeq would require adding type guards or assertions at most of these sites — a significant scope expansion for this PR.
The current state is pragmatically correct: the type says Scalar, the comment says "can be a sequence at runtime," and getValueFromValueNode handles both. Worth cleaning up in a follow-up, but not here.
| stepType: step.stepType, | ||
| scope, | ||
| propertyKey: key, | ||
| values: buildStepSelectionValues(step), |
There was a problem hiding this comment.
this would rebuild for every step properties.
Would it be possible to hoist this above the loop iteration and reuse its value?
There was a problem hiding this comment.
I don't think it is very likely to have more than one prop with selection configured in the same step. But won't hurt anyway, done here: 525d2a0
| export async function validateCustomProperties( | ||
| customPropertyItems: CustomPropertyItem[] | ||
| ): Promise<CustomPropertyValidationResult[]> { | ||
| clearCache(); |
There was a problem hiding this comment.
isn't this a bit aggressive? This could potentially impact also autocomplete
There was a problem hiding this comment.
Yeap, it is.
Actually, since we now pass all the values in the step definition to selection handlers, we would need to use the entire step definition as a cache key, and caching for selection search would become almost useless.
I think that with these changes, caching selection handlers on our side doesn't make sense anymore, because we don't know what values are being used by the registered steps to search the entries. It would be better to delegate the caching out to the selection handlers of the registered steps (other plugins), as they are the ones who know the values they use to search and resolve selection entries.
But I wanted to do it in a follow-up. This is an intermediate state to make things work properly. I'll explicitly remove the cache on our side in a separate PR (the result will be the same, since it is cleaned on every change), and I will notify all the teams about it.
Does it sound good?
| if (!valueNode) return undefined; | ||
| if (YAML.isScalar(valueNode)) return valueNode.value; | ||
| if ('toJSON' in valueNode && typeof valueNode.toJSON === 'function') return valueNode.toJSON(); | ||
| return (valueNode as { value?: unknown }).value; |
There was a problem hiding this comment.
This branch is reached when the property is an array (YAMLSeq):
| if (current[seg] == null || typeof current[seg] !== 'object') { | ||
| current[seg] = {}; | ||
| } | ||
| current = current[seg] as Record<string, unknown>; |
There was a problem hiding this comment.
would it be possible to use the isRecord typeguard instead of the type casting?
There was a problem hiding this comment.
And fixed here 49c8d0e, we can use the isRecord, but we still need to cast it anyway down below...
...ws_management/public/entities/workflows/store/workflow_detail/utils/build_workflow_lookup.ts
Show resolved
Hide resolved
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
cc @semd |
Summary
issue: https://github.com/elastic/security-team/issues/15982. (Resolves requirement #2)
Enriches
SelectionContextwith stepvaluesso thatPropertySelectionHandlerimplementations (search,resolve,getDetails) can access sibling property values from the current step definition.There are no existing steps using
context.valuesyet — this is a prerequisite for upcoming steps from the Cases team that need to read sibling properties (e.g.owner) to scope their search/resolve logic.What changed
SelectionContext.values— A newvaluesfield ({ config, input }) is populated from the step's YAML properties at the timesearch/resolve/getDetailsare called. Handlers can now read sibling values likecontext.values.input.ownerinstead of having no visibility into the rest of the step.YAML value extraction fix —
getValueFromValueNodenow handles non-scalar YAML nodes (arrays/sequences) via.toJSON(), fixing a bug where array properties likeowner: [securitySolution]appeared asundefinedincontext.values.Example — An example has been implemented in the
examples.externalStepfrom the workflows_examples plugin (test withnode scripts/kibana --dev --run-examples)Demo
Gravacio.de.pantalla.2026-04-02.a.les.15.58.23.mov
Files changed
kbn-workflows/types/v1.ts,latest.tsStepSelectionValuesinterface,valuesfield onSelectionContext, generic type parameters on 4 interfacesworkflows_extensions/.../step_registry/types.tsConfig/Inputschema types intoStepPropertyHandlerfor automatic inferencebuild_workflow_lookup.tsbuildStepSelectionValues()helper + fixedgetValueFromValueNodefor non-scalar nodescollect_all_custom_property_items.ts,get_custom_property_suggestions.tsvalueswhen constructingSelectionContextSTEPS.mdSelectionContexttype definition and documentedcontext.valuesbuild_workflow_lookup.test.ts,validate_custom_properties.test.ts,get_custom_property_suggestions.test.ts