Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/agent-deployment-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ jobs:
oc project
echo ""
echo "--- Namespace resources ---"
oc get all -n ci-testing --no-headers | head -10
oc get all -n ci-testing --no-headers 2>&1 | head -10 || true
echo ""
echo "Cluster connection verified successfully."

Expand All @@ -58,6 +58,7 @@ jobs:
matrix:
agent:
- { name: langgraph-react-agent, dir: agents/langgraph/react_agent }
- { name: langgraph-hitl-agent, dir: agents/langgraph/human_in_the_loop }
env:
API_KEY: ${{ vars.API_KEY }}
BASE_URL: ${{ vars.BASE_URL }}
Expand Down
9 changes: 7 additions & 2 deletions agents/langgraph/human_in_the_loop/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ VALUES_FILE := values.yaml
CONTAINER_CLI := $(shell command -v podman 2>/dev/null || command -v docker 2>/dev/null)
MODEL ?= llama3.1:8b

.PHONY: init re-init env ollama llama-server run-app run-app-fresh run-cli build push build-openshift deploy undeploy test dry-run help
.PHONY: init re-init env ollama llama-server run-app run-app-fresh run-cli build push build-openshift deploy undeploy test test-integration dry-run help

help: ## Show this help
@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf " %-12s %s\n", $$1, $$2}'
Expand Down Expand Up @@ -158,4 +158,9 @@ undeploy: ## Remove deployment from cluster
helm uninstall $(AGENT_NAME)

test: ## Run tests
uv run --extra dev python -m pytest tests/
uv run --extra dev python -m pytest tests/ --ignore=tests/integration

test-integration: ## Run integration deployment test
PYTHONPATH=$$(git rev-parse --show-toplevel)/tests \
uv run --extra dev python -m pytest tests/integration/test_deployment.py \
-v --tb=long --junitxml=results.xml
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Re-export shared integration fixtures so pytest discovers them.
from integration.conftest import cluster_auth, repo_root # noqa: F401
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
from __future__ import annotations

import logging
import os

import pytest
from integration.utils import (
MakeTargetError,
RouteNotFoundError,
get_route,
health_check,
load_agent_name,
run_make,
)

logger = logging.getLogger(__name__)

INTERNAL_REGISTRY = "image-registry.openshift-image-registry.svc:5000"


@pytest.fixture(scope="module")
def agent_dir(repo_root):
return repo_root / "agents" / "langgraph" / "human_in_the_loop"


@pytest.fixture(scope="module")
def agent_name(agent_dir):
return load_agent_name(agent_dir)


def _write_env_file(agent_dir, container_image):
"""Write a .env file so Makefile targets can source it."""
missing = [v for v in ("BASE_URL", "MODEL_ID") if v not in os.environ]
if missing:
pytest.fail(
f"Missing required env vars: {', '.join(missing)}. "
"Set them in the CI workflow or export locally."
)
env_path = agent_dir / ".env"
env_path.write_text(
f"API_KEY={os.environ.get('API_KEY', 'not-needed')}\n"
f"BASE_URL={os.environ['BASE_URL']}\n"
f"MODEL_ID={os.environ['MODEL_ID']}\n"
f"CONTAINER_IMAGE={container_image}\n"
)
return env_path


@pytest.fixture(scope="module")
def deployed_agent(cluster_auth, agent_dir, agent_name):
namespace = cluster_auth["namespace"]
container_image = f"{INTERNAL_REGISTRY}/{namespace}/{agent_name}:latest"
env_path = _write_env_file(agent_dir, container_image)

deployed = False
try:
logger.info("Building image on cluster via build-openshift...")
run_make("build-openshift", cwd=agent_dir, timeout=600)

logger.info("Deploying to cluster...")
run_make("deploy", cwd=agent_dir, timeout=300)
deployed = True

route_url = get_route(agent_name, namespace=namespace)
logger.info("Agent deployed at %s", route_url)

yield route_url

except (MakeTargetError, RouteNotFoundError) as exc:
pytest.fail(f"Deployment failed: {exc}")

finally:
if deployed:
logger.info("Tearing down deployment...")
try:
run_make("undeploy", cwd=agent_dir, timeout=120)
except MakeTargetError:
logger.warning(
"Cleanup failed — manual undeploy may be needed", exc_info=True
)
env_path.unlink(missing_ok=True)


@pytest.mark.integration
def test_health_endpoint(deployed_agent):
route_url = deployed_agent
result = health_check(f"{route_url}/health", retries=12, backoff=5.0)

assert result["status"] == "healthy"
assert result["agent_initialized"] is True
Comment thread
tarun-etikala marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
RouteNotFoundError,
get_route,
health_check,
load_agent_name,
run_make,
)

logger = logging.getLogger(__name__)

AGENT_NAME = "langgraph-react-agent"
INTERNAL_REGISTRY = "image-registry.openshift-image-registry.svc:5000"


Expand All @@ -23,6 +23,11 @@ def agent_dir(repo_root):
return repo_root / "agents" / "langgraph" / "react_agent"


@pytest.fixture(scope="module")
def agent_name(agent_dir):
return load_agent_name(agent_dir)


def _write_env_file(agent_dir, container_image):
"""Write a .env file so Makefile targets can source it."""
missing = [v for v in ("BASE_URL", "MODEL_ID") if v not in os.environ]
Expand All @@ -42,9 +47,9 @@ def _write_env_file(agent_dir, container_image):


@pytest.fixture(scope="module")
def deployed_agent(cluster_auth, agent_dir):
def deployed_agent(cluster_auth, agent_dir, agent_name):
namespace = cluster_auth["namespace"]
container_image = f"{INTERNAL_REGISTRY}/{namespace}/{AGENT_NAME}:latest"
container_image = f"{INTERNAL_REGISTRY}/{namespace}/{agent_name}:latest"
env_path = _write_env_file(agent_dir, container_image)

deployed = False
Expand All @@ -56,7 +61,7 @@ def deployed_agent(cluster_auth, agent_dir):
run_make("deploy", cwd=agent_dir, timeout=300)
deployed = True

route_url = get_route(AGENT_NAME, namespace=namespace)
route_url = get_route(agent_name, namespace=namespace)
logger.info("Agent deployed at %s", route_url)

yield route_url
Expand Down
9 changes: 9 additions & 0 deletions tests/integration/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,18 @@
from pathlib import Path

import httpx
import yaml

logger = logging.getLogger(__name__)


def load_agent_name(agent_dir: str | Path) -> str:
data = yaml.safe_load((Path(agent_dir) / "agent.yaml").read_text())
if not isinstance(data, dict) or "name" not in data:
raise ValueError(f"No 'name' field in {agent_dir}/agent.yaml")
return str(data["name"]).strip()
Comment on lines +17 to +20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate name type/content before returning it

Line 20 currently coerces any YAML value to string (str(data["name"])), so invalid values like null become "None" and can propagate into deployment/route names. Fail fast by requiring a non-empty string.

Suggested fix
 def load_agent_name(agent_dir: str | Path) -> str:
     data = yaml.safe_load((Path(agent_dir) / "agent.yaml").read_text())
     if not isinstance(data, dict) or "name" not in data:
         raise ValueError(f"No 'name' field in {agent_dir}/agent.yaml")
-    return str(data["name"]).strip()
+    name = data["name"]
+    if not isinstance(name, str) or not name.strip():
+        raise ValueError(f"Invalid 'name' value in {agent_dir}/agent.yaml: expected non-empty string")
+    return name.strip()

As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
data = yaml.safe_load((Path(agent_dir) / "agent.yaml").read_text())
if not isinstance(data, dict) or "name" not in data:
raise ValueError(f"No 'name' field in {agent_dir}/agent.yaml")
return str(data["name"]).strip()
def load_agent_name(agent_dir: str | Path) -> str:
data = yaml.safe_load((Path(agent_dir) / "agent.yaml").read_text())
if not isinstance(data, dict) or "name" not in data:
raise ValueError(f"No 'name' field in {agent_dir}/agent.yaml")
name = data["name"]
if not isinstance(name, str) or not name.strip():
raise ValueError(f"Invalid 'name' value in {agent_dir}/agent.yaml: expected non-empty string")
return name.strip()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/utils.py` around lines 17 - 20, The code currently coerces
any YAML value to a string and returns it, which allows invalid values like null
to become "None"; update the logic that reads data["name"] to validate that the
field exists, is an instance of str, and that stripped value is non-empty before
returning it (e.g., get the raw value from data["name"], check isinstance(value,
str), let name = value.strip(), and if not name raise ValueError with a clear
message); replace the current str(data["name"]).strip() return with this
validation and raise behavior so only a non-empty string is returned.



_REDACT_PATTERNS = [
re.compile(r"(API_KEY=)\S+"),
re.compile(r'(apiKey:\s*")[^"]*"'),
Expand Down
Loading