[One Workflow] 🐞 Improving detection of steps in the yaml by yngrdyn · Pull Request #249274 · elastic/kibana

yngrdyn · 2026-01-15T17:10:19Z

Closes https://github.com/elastic/security-team/issues/14310
Close https://github.com/elastic/security-team/issues/14570
Closes https://github.com/elastic/security-team/issues/15440

Summary

This PR fixes bugs related to actions menu+ steps and account for the following behaviour:

Cursor on steps: line → Insert step on the following line
Cursor on comment line → Insert step on the following line (after the comment)
Cursor on empty line → Replace the empty line with the new step
Cursor on existing step → Insert after that step
No cursor → Insert at the end (default behavior)
empty YAML → Step section is created and step is added
flow-style → [] is replaced by selected step
empty items → empty item is replaced by selected step

Technical Changes

Simplified insert_step_snippet.ts by extracting helper functions for better maintainability
Created shared utilities (snippet_insertion_utils.ts) to reduce code duplication between step and trigger insertion

Testing

Added comprehensive test coverage for all cursor position scenarios and edge cases.

…-breaking-the-yaml

…he-yaml' of github.com:yngrdyn/kibana into 14310-bug-step-snippet-inserted-on-same-line-breaking-the-yaml

…-breaking-the-yaml

Copilot

Pull request overview

This PR improves the detection and insertion of workflow steps and triggers in YAML documents, fixing bugs related to cursor position handling and various edge cases when using the actions menu.

Changes:

Enhanced step/trigger insertion logic to handle cursor position on steps:/triggers: line, comment lines, empty lines, and existing steps
Replaced flow-style empty arrays ([]) and empty items with proper step/trigger definitions
Extracted shared utilities for snippet insertion to reduce code duplication

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
workflow_yaml_editor.tsx	Removed redundant null check for yamlDocument before step insertion
snippet_insertion_utils.ts	New shared utility module containing helper functions for snippet insertion logic
insert_trigger_snippet.ts	Refactored to use shared utilities from snippet_insertion_utils.ts
insert_step_snippet.ts	Complete rewrite with improved cursor position handling and edge case support
insert_step_snippet.test.ts	Added comprehensive test coverage for all cursor position scenarios and edge cases

Comments suppressed due to low confidence (1)

src/platform/plugins/shared/workflows_management/public/widgets/workflow_yaml_editor/lib/snippets/snippet_insertion_utils.ts:1

The magic number 10 (allowed range after step) lacks explanation. Consider extracting this to a named constant like MAX_LINES_AFTER_STEP_FOR_CURSOR_INSERT with a comment explaining why this specific value was chosen.

/*

Kiryous

Thanks for working on this! While manually testing, I found that if the cursor is right at the end of a step, it works great; if I put a newline after a step or place the cursor on the same line as "steps:", the snippet gets inserted in the wrong position, or ignores the indent of nested steps.

Please see attached screencasts:

As a user I expect, new step will be inserted as a second step in if.steps, not under "else".

after-steps-before-else.mov

As a user I expect that if I place cursor after "steps:", the new step will be inserted inside this section; and if I added a new line after nested step, I expect new step to appear with the same indentation after the step I focused on currently.

after-else.mov

…-breaking-the-yaml

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Copilot · 2026-02-04T13:11:41Z

+  let document: Document;
+  try {
+    document = yamlDocument || parseDocument(model.getValue());
+  } catch (error) {


The parseDocument call could throw an exception if the YAML is malformed, but the error is caught and silently swallowed at line 460. This makes debugging difficult. Consider logging the error or providing feedback about the parse failure.

Suggested change

} catch (error) {

} catch (error) {

// Swallowing YAML parse errors here makes debugging difficult; log with context.

// eslint-disable-next-line no-console

console.error('Failed to parse YAML document in insertStepSnippet', error);

elasticmachine · 2026-02-04T15:49:06Z

⏳ Build in-progress, with failures

Buildkite Build
Commit: edda587

Failed CI Steps

Test Failures

[job] [logs] Jest Tests #9 / insertTriggerSnippet when triggers section exists but is empty should insert trigger when triggers section has multiple comments
[job] [logs] Jest Tests #9 / insertTriggerSnippet when triggers section exists but is empty should insert trigger when triggers section has multiple comments
[job] [logs] Scout: [ platform / streams_app-serverless-oblt ] plugin / serverless-oblt - Stream data processing - data sources management - should allow adding a new kql data source
[job] [logs] Scout: [ platform / streams_app-stateful ] plugin / should allow adding a new kql data source
[job] [logs] Scout: [ platform / streams_app-serverless-oblt ] plugin / should allow adding a new kql data source
[job] [logs] Scout: [ platform / streams_app-stateful ] plugin / stateful - Stream data processing - data sources management - should allow adding a new kql data source

History

cc @yngrdyn

…-breaking-the-yaml

Kiryous

LGTM! Thanks for addressing edge cases ❤️

Thinking out loud: for future, given we'll have a visual editor, we might consider refactoring this logic into a YAML-AST based approach - e.g. "insert step at path steps[0].steps[1]" rather than relying on Monaco line/column positions. This would make the core logic more testable and reusable across editors. But that's a bigger refactor for another day!

…-breaking-the-yaml

kibanamachine · 2026-04-23T09:48:50Z