mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-11 03:13:51 +00:00
Fix: project health-check always exited with code 0, even when the check failed
The failure paths either returned normally or were swallowed by the catch-all exception handler, so callers (CI, scripts) could not act on the command's verdict. Failures now raise, and the verdict is reported outside the checked region -- where a failure to write the report can no longer be mistaken for a failure of the check itself -- and signalled through the exit code. A FindSymbolTool result without any matches is treated as a failure rather than a warning. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
e6e25d3f86
commit
d9efb5ade8
2 files changed
+27
-17
No files matched your search
@@ -17,6 +17,9 @@ Status of the `main` branch. Changes prior to the next official version change w
|
||||
* CLI:
|
||||
- Fix: `start-mcp-server` help text for `--project-from-cwd` falsely promised a fallback to the CWD, which was
|
||||
removed in v1.0.0 #1773
|
||||
- Fix: `project health-check` always exited with code 0, even when the check failed, so callers
|
||||
(CI, scripts) could not act on its verdict; it now exits with code 1 on failure. A `find_symbol`
|
||||
result without any matches is now reported as a failure rather than as a warning.
|
||||
|
||||
* Tools:
|
||||
- `find_symbol`: Change tool description to improve tool search results in clients that load tools dynamically
|
||||
|
||||
+24
-17
@@ -909,6 +909,12 @@ class ProjectCommands(AutoRegisteringGroup):
|
||||
finally:
|
||||
ls_mgr.stop_all()
|
||||
|
||||
class _HealthCheckFailure(Exception):
|
||||
"""
|
||||
A condition under which the health check is to be considered failed.
|
||||
The exception message is the explanation presented to the user.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
@click.command(
|
||||
"health-check",
|
||||
@@ -939,6 +945,9 @@ class ProjectCommands(AutoRegisteringGroup):
|
||||
os.makedirs(log_dir, exist_ok=True)
|
||||
log_file = os.path.join(log_dir, f"health_check_{timestamp}.log")
|
||||
|
||||
# the check's verdict: the explanation of the failure, or None if the check passed (see below)
|
||||
failure_reason: str | None = None
|
||||
|
||||
with FileLoggerContext(log_file, append=False, enabled=True):
|
||||
log.info("Starting health check for project: %s", project_path)
|
||||
|
||||
@@ -965,10 +974,7 @@ class ProjectCommands(AutoRegisteringGroup):
|
||||
continue
|
||||
|
||||
if not target_file:
|
||||
log.error("No analyzable files found in project")
|
||||
click.echo("❌ Health check failed: No analyzable files found")
|
||||
click.echo(f"Log saved to: {log_file}")
|
||||
return
|
||||
raise ProjectCommands._HealthCheckFailure("No analyzable files found")
|
||||
|
||||
# Get tools from agent
|
||||
overview_tool = agent.get_tool(GetSymbolsOverviewTool)
|
||||
@@ -982,10 +988,7 @@ class ProjectCommands(AutoRegisteringGroup):
|
||||
log.info(f"GetSymbolsOverviewTool returned: {overview_data}")
|
||||
|
||||
if not overview_data:
|
||||
log.error("No symbols found in file %s", target_file)
|
||||
click.echo("❌ Health check failed: No symbols found in target file")
|
||||
click.echo(f"Log saved to: {log_file}")
|
||||
return
|
||||
raise ProjectCommands._HealthCheckFailure(f"No symbols found in target file {target_file}")
|
||||
|
||||
# Extract suitable symbol (prefer class or function over variables)
|
||||
preferred_kinds = {SymbolKind.Class.name, SymbolKind.Function.name, SymbolKind.Method.name, SymbolKind.Constructor.name}
|
||||
@@ -1036,28 +1039,32 @@ class ProjectCommands(AutoRegisteringGroup):
|
||||
pattern_matches = 0
|
||||
|
||||
# Verify tools worked as expected
|
||||
tools_working = True
|
||||
if not find_symbol_data:
|
||||
log.error("FindSymbolTool returned no results")
|
||||
tools_working = False
|
||||
raise ProjectCommands._HealthCheckFailure("FindSymbolTool returned no results")
|
||||
|
||||
if len(find_refs_data) == 0 and pattern_matches == 0:
|
||||
log.warning("Both FindReferencingSymbolsTool and SearchForPatternTool found no matches - this might indicate an issue")
|
||||
|
||||
log.info("Health check completed successfully")
|
||||
|
||||
if tools_working:
|
||||
click.echo("✅ Health check passed - All tools working correctly")
|
||||
else:
|
||||
click.echo("⚠️ Health check completed with warnings - Check log for details")
|
||||
|
||||
# expected failures and unexpected exceptions are reported alike: both mean the project's
|
||||
# tooling is not functional, which is the single thing this command is asked to determine
|
||||
except Exception as e:
|
||||
log.exception("Health check failed with exception: %s", str(e))
|
||||
click.echo(f"❌ Health check failed: {e!s}")
|
||||
failure_reason = str(e)
|
||||
|
||||
finally:
|
||||
click.echo(f"Log saved to: {log_file}")
|
||||
|
||||
# the verdict is reported outside the checked region, so that a failure to write the report
|
||||
# cannot be mistaken for a failure of the check itself; the exit code lets callers
|
||||
# (CI, scripts) act on the verdict
|
||||
if failure_reason is None:
|
||||
click.echo("✅ Health check passed - All tools working correctly")
|
||||
else:
|
||||
click.echo(f"❌ Health check failed: {failure_reason}")
|
||||
raise SystemExit(1)
|
||||
|
||||
|
||||
class ToolCommands(AutoRegisteringGroup):
|
||||
"""Group for 'tool' subcommands."""
|
||||
|
||||
Reference in new issue
Block a user