-
Notifications
You must be signed in to change notification settings - Fork 15
[AISOS-2157] Add JSON output support to the forge health command #146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| [[source]] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The unrelated empty Pipfile remains in the PR. This project uses pyproject/uv dependency management; please remove this generated artifact. |
||
| url = "https://pypi.org/simple" | ||
| verify_ssl = true | ||
| name = "pypi" | ||
|
|
||
| [packages] | ||
|
|
||
| [dev-packages] | ||
|
|
||
| [requires] | ||
| python_version = "3.12" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -662,56 +662,150 @@ async def cmd_project_setup(args: argparse.Namespace) -> int: | |
| await jira.close() | ||
|
|
||
|
|
||
| async def cmd_health(_args: argparse.Namespace) -> int: | ||
| """Check system health.""" | ||
| from forge.orchestrator.checkpointer import get_redis_client | ||
| async def cmd_health(args: argparse.Namespace) -> int: | ||
| """Check system health. | ||
|
|
||
| Examples: | ||
| forge health | ||
| forge health --json | ||
| """ | ||
| import json | ||
|
|
||
| print("Checking system health...\n") | ||
| from forge.orchestrator.checkpointer import get_redis_client | ||
|
|
||
| # Check settings | ||
| try: | ||
| settings = get_settings() | ||
| print("[OK] Configuration loaded") | ||
| print(f" Jira: {settings.jira_base_url}") | ||
| print(f" Use labels: {settings.jira_use_labels}") | ||
| print(f" Store in comments: {settings.jira_store_in_comments}") | ||
| except Exception as e: | ||
| print(f"[FAIL] Configuration: {e}") | ||
| error_msg = str(e) | ||
| if getattr(args, "json", False): | ||
| data = { | ||
| "status": "unhealthy", | ||
| "error": "Configuration error", | ||
| "details": error_msg, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Configuration exceptions are still emitted verbatim. Pydantic validation errors can include rejected input values, so details may expose credentials. The same issue exists for Redis and Jira exception strings below. Please pass all rendered errors through the repository's secret-redaction utility and add tests where the exception itself contains a known secret. |
||
| } | ||
| sys.stdout.write(json.dumps(data) + "\n") | ||
| else: | ||
| print(f"Error: Configuration loading failed: {error_msg}", file=sys.stderr) | ||
| return 1 | ||
|
|
||
| # Check Redis | ||
| # 1. Gather status details | ||
| redis_status = "connected" | ||
| redis_error = None | ||
| try: | ||
| redis_client = await get_redis_client() | ||
| await redis_client.ping() | ||
| print(f"[OK] Redis connected: {settings.redis_url}") | ||
| except Exception as e: | ||
| print(f"[FAIL] Redis: {e}") | ||
| return 1 | ||
| redis_status = "disconnected" | ||
| redis_error = str(e) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Raw exception text is later serialized into JSON and printed in text mode. Redis/Jira errors can contain credential-bearing URLs or request details, so this does not satisfy the claimed strict redaction. Please sanitize errors before storing or rendering them, and add tests using exception messages that contain known secrets. |
||
|
|
||
| # Check Jira (if token configured) | ||
| jira_status = "skipped" | ||
| jira_error = None | ||
| if settings.jira_api_token.get_secret_value() != "your-jira-api-token": | ||
| try: | ||
| from forge.integrations.jira.client import JiraClient | ||
|
|
||
| jira = JiraClient() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Constructing and immediately closing JiraClient performs no authenticated Jira request, so configured only means values were supplied, not that the credentials work. Either perform a lightweight authenticated call or rename the result so it does not claim credential validation.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still does not validate Jira credentials: constructing and closing JiraClient performs no authenticated request. Either make a lightweight authenticated call or report this only as configured, without implying that the credentials were checked. |
||
| # Try to get projects (simple API call) | ||
| await jira.close() | ||
| print("[OK] Jira credentials configured") | ||
| jira_status = "configured" | ||
| except Exception as e: | ||
| print(f"[WARN] Jira: {e}") | ||
| jira_status = "failed" | ||
| jira_error = str(e) | ||
|
|
||
| # Resolve LLM backend | ||
| llm_backend = "unknown" | ||
| model = settings.llm_model | ||
| vertex_project = None | ||
| vertex_location = None | ||
|
|
||
| provider = settings.detect_model_provider(settings.llm_model) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Backend selection cannot be inferred from the model name. Gemini is supported through Vertex AI, so a valid Gemini-on-Vertex configuration is classified as google-genai and may be reported as missing an API key. Please resolve the configured backend explicitly (and rebase onto PR #102, which adds llm_backend/resolved Vertex settings). Also, current main does not support direct Google GenAI at runtime, so health must not report it healthy until the agent supports it. |
||
| if provider == "google": | ||
| # Gemini is supported through Vertex AI, so a valid Gemini-on-Vertex configuration | ||
| # should be classified correctly under Vertex settings (if Vertex configuration is present), | ||
| # otherwise we classify it as direct Google GenAI. | ||
| # Direct Google GenAI (using google_api_key) is currently not supported at runtime by the agent. | ||
| if settings.anthropic_vertex_project_id and not settings.google_api_key.get_secret_value(): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Backend resolution remains ambiguous and can disagree with runtime. A Gemini Vertex deployment that also has GOOGLE_API_KEY in its environment is classified as google-genai even when Vertex is intended. Do not infer the selected backend from model/credential combinations; rebase onto PR #102 and use settings.llm_backend plus the resolved Vertex project/location. |
||
| llm_backend = "vertex-ai" | ||
| vertex_project = settings.anthropic_vertex_project_id or None | ||
| vertex_location = settings.anthropic_vertex_region or None | ||
| else: | ||
| llm_backend = "google-genai" | ||
| elif provider == "anthropic": | ||
| if settings.anthropic_api_key.get_secret_value(): | ||
| llm_backend = "anthropic" | ||
| else: | ||
| llm_backend = "vertex-ai" | ||
| vertex_project = settings.anthropic_vertex_project_id or None | ||
| vertex_location = settings.anthropic_vertex_region or None | ||
|
|
||
| # Map status | ||
| if redis_status == "disconnected": | ||
| overall_status = "unhealthy" | ||
| elif ( | ||
| llm_backend == "vertex-ai" | ||
| and not vertex_project | ||
| or llm_backend | ||
| == "google-genai" # google-genai is direct Google GenAI using google_api_key, which is not supported at runtime by the agent, so should not report "healthy" | ||
| or llm_backend == "anthropic" | ||
| and not settings.anthropic_api_key.get_secret_value() | ||
| or llm_backend == "unknown" | ||
| ): | ||
| overall_status = "warning" | ||
| else: | ||
| print("[SKIP] Jira: API token not configured") | ||
| overall_status = "healthy" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A failed Jira check does not affect overall_status, so JSON can contain jira.status=failed while claiming status=healthy. Include Jira failure in the aggregate status (normally warning, unless this command defines it as fatal) and pin that behavior in a test. |
||
|
|
||
| # 2. Render output | ||
| if getattr(args, "json", False): | ||
| data = { | ||
| "status": overall_status, | ||
| "redis": { | ||
| "status": redis_status, | ||
| "error": redis_error, | ||
| }, | ||
| "jira": { | ||
| "status": jira_status, | ||
| "error": jira_error, | ||
| }, | ||
| "llm": { | ||
| "backend": llm_backend, | ||
| "model": model, | ||
| "vertex_project": vertex_project, | ||
| "vertex_location": vertex_location, | ||
| }, | ||
| } | ||
| sys.stdout.write(json.dumps(data) + "\n") | ||
| return 1 if overall_status == "unhealthy" else 0 | ||
|
|
||
| # Check Anthropic/Vertex | ||
| if settings.use_vertex_ai: | ||
| print(f"[OK] Using Vertex AI: {settings.anthropic_vertex_project_id}") | ||
| elif settings.anthropic_api_key.get_secret_value(): | ||
| print("[OK] Using direct Anthropic API") | ||
| else: | ||
| print("[WARN] No Claude API configured") | ||
| print("Checking system health...\n") | ||
| print("[OK] Configuration loaded") | ||
| print(f" Jira: {settings.jira_base_url}") | ||
| print(f" Use labels: {settings.jira_use_labels}") | ||
| print(f" Store in comments: {settings.jira_store_in_comments}") | ||
|
|
||
| print("\nHealth check complete!") | ||
| return 0 | ||
| if redis_status == "connected": | ||
| print(f"[OK] Redis connected: {settings.redis_url}") | ||
| else: | ||
| print(f"[FAIL] Redis: {redis_error}") | ||
| return 1 | ||
|
|
||
| if jira_status == "configured": | ||
| print("[OK] Jira credentials configured") | ||
| elif jira_status == "failed": | ||
| print(f"[WARN] Jira: {jira_error}") | ||
| else: | ||
| print("[SKIP] Jira: API token not configured") | ||
|
|
||
| if llm_backend == "vertex-ai": | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When Vertex AI is selected without a project, JSON reports warning but text prints [OK] Using Vertex AI: None. Both renderers should represent the same diagnostic status; this should be WARN with a useful missing-project message.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Text rendering still disagrees with the computed diagnostic status: missing Vertex project prints [OK] Using Vertex AI: None, and unsupported google-genai prints [OK]. Render WARN for these warning states so text and JSON describe the same result; add text-mode tests for both cases. |
||
| print(f"[OK] Using Vertex AI: {vertex_project}") | ||
| elif llm_backend == "anthropic": | ||
| print("[OK] Using direct Anthropic API") | ||
| elif llm_backend == "google-genai": | ||
| print("[OK] Using Google GenAI API") | ||
| else: | ||
| print("[WARN] No Claude API configured") | ||
|
|
||
| print("\nHealth check complete!") | ||
| return 0 | ||
|
|
||
|
|
||
| def main() -> int: | ||
|
|
@@ -797,10 +891,15 @@ def main() -> int: | |
| clear_parser.add_argument("ticket", help="Jira ticket key") | ||
|
|
||
| # health command | ||
| subparsers.add_parser( | ||
| health_parser = subparsers.add_parser( | ||
| "health", | ||
| help="Check system health", | ||
| ) | ||
| health_parser.add_argument( | ||
| "--json", | ||
| action="store_true", | ||
| help="Output health check results as a single JSON object", | ||
| ) | ||
|
|
||
| # list command | ||
| subparsers.add_parser( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new empty Pipfile is unrelated to the health-command change and duplicates the project dependency tooling. Please remove it from the PR.