Regression: simple pipeline fail

[shenxianpeng]

this fail on simple pipeline. @shenxianpeng We should keep patch for a while in review, ask at least to be tested and then merge. Even before to create such patch we should have an issue open, discuss the implementation and review it to check if it can have problems in it.

Originally posted by @panicking in #99 (comment)

[panicking]

@shenxianpeng we have some problems:

1. Strategy 3 that is implemented (I miss strategy 2) is alway executed, when should not, if we found a node that fail or multiple node that fail we should look for the link of common ancestor if we really make importance to AI review. Even AI is not informed about how log are splitted, there is no way to explain that each log is part of a different flow of execution. Then strategy 3 can not always run on all the rest because is already done by getResult with pattern.
2. I found out that plugin that generate Warning can result in a fail of the pipeline so this needs to be parsed.

I have a fix for 2 and a partial revert of 1

[panicking]

@shenxianpeng now the result make sense

The explanation is about the node and then link is correct. If I don't drop 3 then explanation is wrong and link is correct. The problem is that hard coded a patter in point 3 has limited sense for now. We should really instruct the AI that we are talking about different path of flow of execution instead concatenated the log

[panicking]

@shenxianpeng Please try to rebase force-forward, this allow to do some git blame for largest patcheset that get merged

[panicking]

For Node information I have this patch

0001-PipelineLogExtractor-Let-AI-distinguish-log-by-FlowN.patch

[lidiams96]

Hi @panicking,

Thank you for the detailed feedback and the node identification patch — that improvement looks great and helps the AI understand the context of each log segment.

Regarding Strategy 3, I want to share the specific production scenario that required it to be always-on rather than fallback-only.

In a pipeline with both parallel branch failures and a catchError + sh(returnStatus:true) + error() pattern, Strategy 1 (multi-collect) behaves as follows:

1. It finds the parallel branch failure nodes — these have ErrorAction and LogAction → logs are collected
2. It finds the error() step node — this has ErrorAction but no LogAction (it only carries the message string) → skipped

Since Strategy 1 returns a non-empty result, a fallback-only Strategy 3 would never run. The catchError block output — which is the actual root cause — would be missed entirely.

Strategy 3 as always-on supplements Strategy 1's results and surfaces those missing logs via pattern matching, with deduplication to avoid repeating lines already captured.

Reverting Strategy 3 to fallback-only would be a regression for this case. Happy to discuss alternatives that address the noise issue for simple pipelines without breaking this scenario.

[panicking]

Hi @panicking,

Thank you for the detailed feedback and the node identification patch — that improvement looks great and helps the AI understand the context of each log segment.

Regarding Strategy 3, I want to share the specific production scenario that required it to be always-on rather than fallback-only.

In a pipeline with both parallel branch failures and a catchError + sh(returnStatus:true) + error() pattern, Strategy 1 (multi-collect) behaves as follows:

1. It finds the parallel branch failure nodes — these have ErrorAction and LogAction → logs are collected
2. It finds the error() step node — this has ErrorAction but no LogAction (it only carries the message string) → skipped

Since Strategy 1 returns a non-empty result, a fallback-only Strategy 3 would never run. The catchError block output — which is the actual root cause — would be missed entirely.

Strategy 3 as always-on supplements Strategy 1's results and surfaces those missing logs via pattern matching, with deduplication to avoid repeating lines already captured.

Reverting Strategy 3 to fallback-only would be a regression for this case. Happy to discuss alternatives that address the noise issue for simple pipelines without breaking this scenario.

First of all if you go to the standard flow, you can add a regular expression then, if you are not failing in the node where the pipeline generate the error, I think you should fix it. Do you have a pipeline-graphview output? You should have red flownodes and then you need to select logs for the first parents of them in case but forget about your regex because you can not make any assumption about what kind of application is running. Another problem of the fallback is that if you have already found a node or more with log, you then could provide invalid info. In my case your fallback totally miss understand the reason because the fallback inject just noise that was just try to interprete linting output as error. For a moment don't use cloude, we should have consistent change that make sense and collaborate to understand how think can be solved.

[lidiams96]

Hi @panicking,

Thank you for the clarification — the regex approach is indeed fragile and your concern about linting output being misinterpreted is completely valid. Navigating the FlowGraph to find the enclosing block of the failing node is a cleaner and more reliable solution.

One question: in the catchError + sh(returnStatus:true) + error() pattern, the red node is error() which has no LogAction. The relevant log is in the sh step, which is a sibling within the catchError block. Does your patch handle traversal into the children of the enclosing block to find that log?

Regarding the use of Claude — it's just a tool for assistance. The PR was opened based on real production testing with a complex parallel pipeline, and the conclusions shared here reflect actual observed behaviour, not generated assumptions. Happy to collaborate on a proper solution.

[panicking]

Hi @panicking,

Thank you for the clarification — the regex approach is indeed fragile and your concern about linting output being misinterpreted is completely valid. Navigating the FlowGraph to find the enclosing block of the failing node is a cleaner and more reliable solution.

One question: in the catchError + sh(returnStatus:true) + error() pattern, the red node is error() which has no LogAction. The relevant log is in the sh step, which is a sibling within the catchError block. Does your patch handle traversal into the children of the enclosing block to find that log?

Regarding the use of Claude — it's just a tool for assistance. The PR was opened based on real production testing with a complex parallel pipeline, and the conclusions shared here reflect actual observed behaviour, not generated assumptions. Happy to collaborate on a proper solution.

Hi, what I think really is that you should fix the pipeline. If you really want a script executed outside try catch , then it means that your flownode will be green and it means that when you generate the error then the error message must be in the log and contains information why it fail. We can not solve general problem. Adding step three is not needed just pass to the fallback a stripped log based on assumptions that the parent will have some interesting log, but this should be done only for sh one that fail. Still thinking that fix the pipeline is the right way. Not always a script that fail us wrong but I don't want to be parsed by AI, maybe it's a must fail in the logic.

[panicking]

@lidiams96 I have implemented the findind of common ancestor if more then one node are failing and provide the link of the parent node

[lidiams96]

Great progress @panicking! Looking forward to the PR.

[panicking]

Great progress @panicking! Looking forward to the PR.

The pr exist ;)

[lidiams96]

Hi @panicking,

Looking at PR #106, we noticed that removing Strategy 3 introduces a regression for the catchError + sh(returnStatus:true) + error() pattern. The new WarningAction path in Strategy 1 sets origin to the catchError BlockStartNode, which has no LogAction → skipped. The sh step is green (no ErrorAction, no WarningAction) → also invisible. The result falls back to run.getLog(maxLines), same as before PR #99.

We fully agree that the regex approach was fragile. Before opening a PR, we wanted to check if the following FlowGraph-based approach would be acceptable to you:

When Strategy 1 finds an error() node with ErrorAction but no LogAction, instead of skipping it, walk up to its enclosing block (the catchError BlockStartNode) and search among its child nodes for a sh step that has a LogAction. No regex, pure graph traversal.

Would this direction make sense to you?