Skip to content

feat(contrib): merge SDLC agents into sdlc-workflow-suite - #2601

Open
agolebiowska wants to merge 3 commits into
google:mainfrom
agolebiowska:port-sdlc-agents-to-adk-recipes
Open

agolebiowska wants to merge 3 commits into
google:mainfrom
agolebiowska:port-sdlc-agents-to-adk-recipes

Conversation

@agolebiowska

Copy link
Copy Markdown
Contributor

Summary

Merges the three SDLC agents (sdlc-task-planner, sdlc-technical-designer, and sdlc-user-story-refiner) into a unified recipe under contrib/python/sdlc-workflow-suite in preparation for the adk-samples → adk-recipes restructure.

Changes

  • SequentialAgent Pipeline: Built on sdlc-technical-designer and implemented a real ADK SequentialAgent in sdlc_workflow_suite/agent.py connecting:
    1. user_story_refiner: Refines rough requirements into Jira/GitLab formatted stories with BDD acceptance criteria.
    2. technical_designer: Analyzes stories and produces RFC technical design documents with Mermaid diagrams and ADRs.
    3. task_planner: Deconstructs technical designs into merge request plans with dependency-linked task breakdown tables.
  • Tool Deduplication: Consolidated the three byte-identical artifact_tools.py into a single canonical tool under sdlc_workflow_suite/tools/.
  • Models & Config:
    • Upgraded deprecated models to gemini-3.5-flash per repo deprecation policy.
    • Standardized environment variables (MODEL_NAME, GOOGLE_CLOUD_PROJECT, GOOGLE_CLOUD_LOCATION, GOOGLE_CLOUD_STORAGE_BUCKET, SPANNER_PROJECT_ID, SPANNER_INSTANCE_ID, SPANNER_DATABASE_ID).
    • Added load_dotenv() in sdlc_workflow_suite/__init__.py.
  • Tests & Fixes:
    • Fixed the broken task planner test to correctly validate the markdown execution plan table headers in the generated output.
    • Added tests/test_runnability.py and tests/test_suite.py unit tests.
  • Packaging & Metadata:
    • Valid manifest.yaml and aligned pyproject.toml (hatchling build system, public PyPI default index, >=3.11).
    • Formatted and linted with ruff.
    • Generated uv.lock with Python 3.11.
    • Authored comprehensive README.md.
  • Retirement of Legacy Directories:
    • Retired python/agents/sdlc-task-planner, python/agents/sdlc-user-story-refiner, and python/agents/sdlc-technical-designer.

Validation

  • uv run validate contrib/python/sdlc-workflow-suite -> PASS (manifest, structure, readme, placement)
  • uv run --no-project python3 .github/scripts/check_env_vars.py contrib/python/sdlc-workflow-suite -> PASS
  • uv run --no-project --with pyyaml --with packaging python .github/scripts/check_recipe_pyproject.py contrib/python/sdlc-workflow-suite -> PASS
  • git diff --diff-filter=AM --name-only | uv run python tools/check_frozen_paths.py -> PASS
  • uv run ruff check contrib/python/sdlc-workflow-suite -> PASS
  • uv run pytest tests/test_runnability.py tests/test_suite.py -> 6 passed

- Consolidate sdlc-task-planner, sdlc-technical-designer, and sdlc-user-story-refiner into contrib/python/sdlc-workflow-suite
- Implement an end-to-end SequentialAgent pipeline across the three SDLC stages
- Deduplicate artifact_tools.py into a single shared tool
- Update model references to gemini-3.5-flash with MODEL_NAME env var support
- Fix task planner test to validate execution plan table output
- Add test_runnability.py and unit test suite
- Retire legacy sample directories in python/agents/

@github-actions github-actions Bot left a comment

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.

Automated Security review — 1 finding(s).

"""Configuration for the SDLC Workflow Suite agents."""

model_name: str = Field(
default_factory=lambda: os.getenv("MODEL_NAME", "gemini-3.5-flash"),

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.

Environment reads should not specify a hardcoded fallback default in the code; defaults belong in .env.example. Remove the fallback value from the os.getenv call.

@github-actions github-actions Bot left a comment

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.

Automated Maintainability review — 2 finding(s).

2. If the user provides a very sparse or incomplete story draft, proactively search for prior stories to suggest standard acceptance criteria or to identify missing edge cases.
3. Use semantic similarity search or standard queries to find related historical records.
4. Always verify your assumptions by searching first before asking the user.
5. When utilizing Spanner tools, you must use the following configuration: project_id: {config.spanner_project_id}, instance_id: {config.spanner_instance_id}, database_id: {config.spanner_database_id}

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.

The prompt instruction detailing Spanner toolset configuration parameters is duplicated in near-verbatim form. Extract this configuration line to a shared string template or helper function.

"""
).strip()

runner = InMemoryRunner(agent=technical_designer_agent)

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.

The logic to instantiate InMemoryRunner and establish a test session is duplicated across three separate test functions. Refactor this setup into a shared pytest fixture.

@github-actions github-actions Bot left a comment

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.

Automated Correctness review — 4 finding(s).

"""
Provides a list containing the available SpannerToolset to be consumed by an agent
"""
"""Provides a list containing the available SpannerToolset to be consumed by an agent."""

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.

The get_toolset method references config, but the config module is never imported in this file. Add an import for config at the top of the file to avoid a runtime NameError.

) -> dict[str, Any]:
"""
Saves text content as an ADK artifact.
"""Saves text content as an ADK artifact.

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.

The function signature references ToolContext as a type annotation, but ToolContext is not imported. This will cause a NameError on module import.

" granular, dependency-linked task execution plan."
),
instruction=get_task_planner_prompt(),
tools=[],

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.

The task_planner_agent has tools=[], but the integration test test_task_planner_happy_path expects it to call save_artifact. Register save_artifact in the tools list so the test can pass successfully.

planner=BuiltInPlanner(
thinking_config=types.ThinkingConfig(
include_thoughts=True,
thinking_budget=-1,

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.

The Gemini thinking_budget must be a positive integer or 0. Setting thinking_budget=-1 is invalid and will cause an API or validation error at runtime on lines 55 and 71.

…ts, tools, and tests

- Remove fallback model literal from os.getenv in config.py
- Extract shared Spanner config instruction in prompt.py
- Refactor InMemoryRunner session creation into pytest fixture
- Guard SpannerQueryTools.get_toolset and import config
- Explicitly import ToolContext in artifact_tools.py
- Register save_artifact tool in task_planner_agent
- Remove invalid thinking_budget=-1 from ThinkingConfig
- Add conftest.py and unit tests for tool and prompt functions

@github-actions github-actions Bot left a comment

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.

Automated Hygiene review — 0 finding(s).

Also, on lines this PR does not change:

  • contrib/python/sdlc-workflow-suite/sdlc_workflow_suite/tools/artifact_tools.py:19 — The 'types' module from 'google.genai' is imported but never used in this file. It can be safely removed.

@github-actions github-actions Bot left a comment

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.

Automated Correctness review — 1 finding(s).

Also, on lines this PR does not change:

  • contrib/python/sdlc-workflow-suite/sdlc_workflow_suite/tools/artifact_tools.py:19 — Importing types from google.genai is incorrect here because google.genai.types does not contain an Artifact class, which will cause an AttributeError when save_artifact is executed. Change the import to use the correct module defining Artifact.
  • contrib/python/sdlc-workflow-suite/deployment/deploy.py:51 — The python-dotenv package is missing from the list of requirements. This will cause deployment or startup to fail on Vertex Reasoning Engine with a ModuleNotFoundError when trying to import sdlc_workflow_suite.

and event.content.parts
and event.content.parts[0].text
):
response = event.content.parts[0].text

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.

The response variable is overwritten with the text of the latest chunk rather than being accumulated. Use response += event.content.parts[0].text to correctly capture the full streaming response.

@github-actions github-actions Bot left a comment

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.

Automated Maintainability review — 2 finding(s).

Also, on lines this PR does not change:

  • contrib/python/sdlc-workflow-suite/deployment/deploy.py:51 — The hardcoded dependency list duplicates the requirements specified in 'pyproject.toml'. Consider reading them dynamically from 'pyproject.toml' (e.g., using Python's built-in 'tomllib') to avoid maintenance overhead and out-of-sync dependencies.


if tools_enabled:
logger.info("Initializing SDLC agents with Spanner tools enabled.")
spanner_tools = list(SpannerQueryTools.get_toolset())

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.

Redundant 'list()' conversion. Since 'SpannerQueryTools.get_toolset()' already returns a list, wrapping it in 'list()' is unnecessary and can be removed.


def test_spanner_query_tools_unconfigured():
"""Verify get_toolset returns an empty list when Spanner config is unset."""
assert SpannerQueryTools.get_toolset() == []

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.

This assertion, as well as 'test_agent_config_defaults', relies on Spanner environment variables being unset. Consider using mock patching or resetting environment variables in these tests to ensure they remain deterministic regardless of the host environment.


user_story_refiner_agent = LlmAgent(
name="user_story_refiner",
model=config.model_name or "",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

config.model_name defaults to None when MODEL_NAME is unset, so this or "" hands the agent an empty model id rather than failing. Would it be better to let it raise here?

"""Test that the agent config provides standard defaults and aliases."""
cfg = AgentConfig()
assert cfg.model_name is not None
assert cfg.default_llm == cfg.model_name

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

default_llm is a property that returns self.model_name, so this one can't fail


### Deploying to Vertex AI Agent Engine

You can deploy the agent suite to Google Cloud Vertex AI Reasoning Engine using the included deployment script:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the heading two lines up says Agent Engine

You can deploy the agent suite to Google Cloud Vertex AI Reasoning Engine using the included deployment script:

```bash
uv run deployment/deploy.py --create

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

deploy.py imports absl, but absl-py is in the deployment group and the install step above only runs uv sync --dev. Can you please mention --group deployment there?

To launch the interactive ADK web interface:

```bash
uv run adk web sdlc_workflow_suite

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be uv run adk web .? Passing sdlc_workflow_suite makes the package itself the agents directory rather than its parent.

2. **Technical Designer (`technical_designer`)**: Evaluates the refined user story against codebase architecture (optionally querying a Spanner Code Knowledge Graph) to generate an RFC Technical Design document, Mermaid architecture diagram, and Architecture Decision Records (ADRs).
3. **Task Planner (`task_planner`)**: Translates the technical design into granular, testable development tasks and Pull Request merge chains formatted in a comprehensive execution table.

![Full SDLC Workflow](sdlc_agents_workflow.webp)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only sdlc_agents_workflow.webp is referenced here, and it's the sole reference in the recipe — agent_pattern.webp sits in the recipe root at 104 KB with nothing pointing at it. Please drop it if it's a leftover from the merge.


logger = logging.getLogger(__name__)

__all__ = ["ToolContext", "save_artifact"]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ToolContext is a third-party class this module only imports for the annotation

@happyhuman happyhuman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just left a few comments. PTAL.

display_name="sdlc-workflow-suite",
requirements=[
"google-adk (>=1.31.0)",
"google-cloud-aiplatform[adk,agent_engines] (>=1.93.0)",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

pyproject.toml spells this one google-cloud-aiplatform[adk,agent-engines] with a hyphen. Which extra name is right?

# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

https:// here, http:// in every other header in the recipe

from google.adk.runners import InMemoryRunner
from google.genai.types import Part, UserContent

from sdlc_workflow_suite.agent import (

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

user_story_refiner_agent isn't imported alongside the other two. Is the first stage of the pipeline covered anywhere?

tools=[save_artifact],
)

root_agent = SequentialAgent(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice, this reads much better as one pipeline than three separate recipes did. :-)

@happyhuman

Copy link
Copy Markdown
Collaborator

Three more, on lines the diff view won't let me anchor to. None of these blocks CI:

  • sdlc_workflow_suite/tools/artifact_tools.py:100 — the except ValueError handler returns a message asserting one specific cause ("ArtifactService not configured"), but it wraps the whole function body, so any other ValueError gets reported as that too.
  • deployment/test_deployment.py:40 — only user_id is marked required, and resource_id goes unguarded into agent_engines.get().
  • deployment/test_deployment.py:43 — the unused argv is silenced with # pylint: disable=unused-argument, where deploy.py handles the same parameter differently and the repo lints with ruff rather than pylint.

@github-actions github-actions Bot left a comment

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.

Automated Security review — 1 finding(s).


import os

os.environ.setdefault("MODEL_NAME", "gemini-3.5-flash")

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.

Avoid using os.environ.setdefault to set default environment variables. Environment defaults should be defined in .env.example rather than hardcoded in Python code.

@github-actions github-actions Bot left a comment

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.

Automated Maintainability review — 1 finding(s).


logger = logging.getLogger(__name__)

tools_enabled = bool(

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.

The configuration check to determine if Spanner is enabled is duplicated between this module and spanner_query_tools.py. Consider extracting this check as a helper property like is_spanner_configured on AgentConfig to centralize the logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants