Dart commands that return non zero exit codes shouldn't set isError to true.

[gspencergoog]

Recently, I've had a lot of issues with the run_tests tool in Gemini CLI where the LLM interprets the "isError" field to mean that the test tool failed to run, rather than that tests have failed. It basically keeps trying to re-run "run_tests" because it thinks there is an error in parsing the test output. It makes the run_tests tool useless.

I think that a non-zero exit code for things like dart test, dart format, etc. shouldn't be interpreted as a "failure" of the tool. The LLM doesn't get to see the "failureReason", so it can't tell the difference between missing roots or some other failure and a failed test.

I think that the isError should only be set if the tool actually failed to run.

The output of "run_tests" would normally be expected to include test failures. We use the error exit code to inform CI and Unix tools that there were test failures, but even then they can't tell the difference between a path error and a test failure.

It's unlikely that the LLM will interpret a tool that doesn't have isError set as having all passing tests in the presence of test output showing failures.

As a data point, the analyze_files tool doesn't set isError when there are analyzer errors, and the LLM can interpret that perfectly well.

[gspencergoog]

I proposed a PR to fix this.

I realize that this might also cause other issues: 1) we'd no longer collect analytics on the command (e.g. test) failures (but I don't think those are actually useful to us), and 2) There are cases where the tools fail with a non-zero status and it's NOT because the tests fail, but because they have a problem accessing a path or some other actual failure. I think the LLM can still interpret that output, but it's definitely a "tool failure" from its point of view. Short of adding a dart test or dart format flag that returns a zero exit even if tests fail or files are formatted, I don't see a good way to differentiate these.