agents/gcp_solution_architecture_agent
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs: design the portable solution architect agent
Some checks failed
validation / verify (push) Failing after 10s
Details

Browse Source 
Record the direction and build plan for replacing the upstream
google-cloud-solution-architecture skill with a portable agent.

- ADR-0002: Agent Skills are the portable unit of behaviour, loaded by
  the framework's native skill runtime; discovery and grounding use
  each Cloud provider's hosted remote MCP servers.
- ADR-0003: LangGraph holds Execution state, checkpoints and
  interrupts; a ReAct Orchestrator and Specialists run as ADK
  LlmAgents with SkillToolset. Supersedes ADR-0001.
- CONTEXT.md: domain glossary (Execution, Phase, Approval, Revision,
  Orchestrator, Specialist, Deliverable, Dependency, Cloud provider).
- .scratch/solution-architect-agent/spec.md: build spec with the
  dependency graph, Revision rules, tool tiers, MCP allowlists, A2A
  interaction, tests and nine build increments.
- .scratch/phase-pipeline/spec.md: superseded; kept as decision log.
- CLAUDE.md and docs/agents/: agent skill configuration (local
  markdown issue tracker, triage labels, domain docs).
- README.md: direction note; flags current ADK classes as stubs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Jonathan Boniface
2026-09-15 18:09:26 +01:00
parent a24a44e28c
commit 4fefe0ab33
11 changed files with 1081 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

361
.scratch/phase-pipeline/spec.md Normal file
View File

@@ -0,0 +1,361 @@
# Spec: Phase pipeline
Status: superseded by [`.scratch/solution-architect-agent/spec.md`](../solution-architect-agent/spec.md). Don't implement. This file remains the decision log (D1 to D46), and its still-valid cleanup items moved to increment 9 of the new spec.
Collapse the three orchestrations of the five Phases into one deep **Phase pipeline** module on LangGraph, with a single phase table as the source of truth.
- Origin: architecture review candidate 01 (2026-09-15), settled through a grilling session.
- Glossary: [`CONTEXT.md`](../../CONTEXT.md). Decision records: [ADR-0001](../../docs/adr/0001-langgraph-orchestrates-phases-not-google-adk.md) (reopened), [ADR-0002](../../docs/adr/0002-skills-are-the-portable-unit-of-behaviour.md).
## Reopened by ADR-0002
The agent is meant to replace the upstream [`google-cloud-solution-architecture`](https://github.com/google/skills/blob/main/skills/cloud/google-cloud-solution-architecture/SKILL.md) skill. It should run Agent Skills through a framework's own skill runtime (ADK `SkillToolset`, or Deep Agents `skills=`), stay portable across Cloud providers, and target Google Cloud first. The decisions below were made before that goal was stated.
| Decision | Why it's in question |
|---|---|
| D1 LangGraph `StateGraph` sequences Phases | If the Skill describes the workflow and a skill runtime executes it, code-owned sequencing duplicates the Skill. The framework choice shrinks to "which skill runtime first". Note that Deep Agents compiles to a LangGraph graph, so the KAB workflow standard can still be met. |
| D2 Phase bodies stay deterministic templates | A Skill's instructions only matter if a model executes them. Templates bypass the Skill entirely. |
| D4 No approval gate | The upstream Skill requires explicit user approval of every Deliverable, and permission before running any script. |
| D6 Phase row binds one Skill per Phase | The upstream Skill is one Skill covering all four Phases, with `references/` and `assets/`. It sits badly with one-Skill-per-Phase, and our `app/skills/*` don't follow the specification (snake_case names, custom `phase:` key). |
| D7 `step(state, ctx)` with an injected `skill_prompt` | Skill runtimes use progressive disclosure: the model calls `load_skill` / `load_skill_resource`. They don't take prompt injection. |
| D5, D11 to D13, D15, D16 | These depend on who sequences Phases. |
| Phase list | Upstream has four Phases, with current state folded into requirements discovery. Ours has five, with a separate `source_discover`. |
### Settled so far (round 5)
| # | Decision |
|---|---|
| D19 | **Hybrid sequencing.** Code owns Phase boundaries and the Approvals between them. The Skill, run through a skill runtime, owns the work within each Phase. |
| D20 | **Interactive by default**, with multi-turn Approvals. A pre-approved mode serves batch and eval runs. The `/generate` one-shot contract (D3) is no longer the primary interface. |
| D21 | **Vendor the upstream Skill** unmodified and pinned to a commit. Our tooling gets its own Skills. The five `app/skills/*` are retired. |
| D22 | **A Cloud provider is a configured bundle of Skills and tools.** No provider interface yet, and no provider prefix on core module names. |
| D23 | **Not linear.** A hierarchy of agents follows the dependencies between Deliverables, and every agent runs a ReAct loop. This supersedes the linear phase table (D5, D6) and the round 6 questions on the engine (Q23), Approval granularity (Q25) and Phase completion (Q26), which are re-asked once the hierarchy is settled. |
### Settled in rounds 6 and 7
| # | Decision |
|---|---|
| D24 (Q23) | **Engine.** LangGraph holds Execution state, checkpoints and `interrupt()`. The Orchestrator and Specialists are ADK `LlmAgent`s with `SkillToolset`, behind one agent-runtime interface. Deep Agents is a later second implementation. See ADR-0003. |
| D25 (Q24) | **Upstream's 4 Phases**, as groupings of Deliverables. The environment scan becomes a tool the requirements analyst can call. |
| D26 (Q25) | **Approvals.** Code enforces and records Approval at Phase boundaries. Within a Phase, approval is conversational and driven by the Skill. Every user question is an `interrupt()`. |
| D27 (Q26) | **Completion.** An agent finishes its work by calling a submit tool, and code checks the submission against the Deliverable contract. If anything is missing, the tool tells the agent what. |
| D28 (Q27) | **Transport.** A2A for interactive Executions (`input-required` covers questions and Approvals). `/generate` is kept for pre-approved Executions. |
| D29 (Q28) | **Hierarchy.** An Orchestrator sits above one Specialist per Deliverable. A Specialist fans out only for independent parts, such as the 6 WAF pillar advisors under the design advisor. |
| D30 (Q29) | **Dependency graph declared in code.** The ReAct Orchestrator chooses work through tools (`ready_work`, `delegate`, `request_approval`) that refuse work whose dependencies aren't met. Independent work may run in parallel. |
| D31 (Q30) | **Per-agent ReAct step budgets**, configurable. Hitting a budget hands control back to the user with a summary. |
### Settled in round 8
| # | Decision |
|---|---|
| D32 (Q31) | **Two thresholds.** A Dependency inside a Phase is met once the Deliverable is submitted, and a Specialist submits only after the user's conversational OK. A Dependency across Phases is met only by that Phase's Approval. |
| D33 (Q32) | **One voice.** Only the Orchestrator talks to the user. A Specialist asks through `needs_input(question)`, and questions from parallel Specialists are combined. |
| D34 (Q33) | **Stateless Specialists.** Each delegation starts fresh from its Dependencies, any feedback and the user's answers. Only the LangGraph checkpoint persists. |
| D35 (Q34) | **Revision replaces the Review round.** Reopening a Deliverable returns it and everything that depends on it to draft, and revokes affected Phase Approvals. Revisions per Deliverable are capped, and at the cap the Orchestrator escalates to the user. |
| D36 (Q35) | **Models are configurable per agent role** via ADK LiteLLM, with a Gemini default. |
| D37 (Q36) | **New spec** at `.scratch/solution-architect-agent/spec.md`. This spec becomes superseded, and its still-valid parts become a cleanup section there. |
### Settled in round 9
| # | Decision |
|---|---|
| D38 (Q37) | **The dependency graph** is as proposed: 9 Deliverables across 4 Phases, deployment guidance depends on design recommendations, and runtime validation is out of scope for v1. |
| D39 (Q38) | **Shared `SkillToolset` plus a role file per agent** (`agents/<role>.md`, framework-neutral). Pillar advisors are scoped to their own WAF Skill. |
| D40 (Q39) | **Vendor the upstream solution-architecture Skill and the 6 WAF Skills** into `skills/vendor/google/`, pinned to `81a31a6`, with `VENDOR.md`, the Apache-2.0 licence and a sync script. |
| D41 (Q40) | **Tool tiers.** Offline tools run freely. Tools that read live cloud resources need permission once per Execution (`allow_cloud_read` in pre-approved mode). Nothing that changes cloud resources is exposed, and `run_skill_script` is disabled. |
| D42 (Q41) | **Deliverables** live in Execution state and the DB, and are written only to `deliverables/executions/{id}/` after Phase 4 Approval. No shared-path copies. |
| D43 (Q42) | **A fake agent runtime** for deterministic tests. The real-model eval runs pre-approved with scripted user answers. |
| D44 (Q43) | **Build alongside the current code**, then one cleanup increment after the eval passes. |
| D45 | **Discovery and grounding use the Cloud provider's own remote MCP servers** (user direction, 2026-09-15). The servers are declared in the provider bundle, with reviewed read-only allowlists. All customer-reading MCP tools are `cloud_read`. `GCPEnvironmentScanner` and the hand-rolled Developer Knowledge client are retired. |
| D46 | **Call remote MCP servers directly**, with no platform MCP gateway for now (user decision, 2026-09-15). |
The spec needs rewriting around the Orchestrator and Specialists once the open questions are settled. The current design section still describes the superseded linear Phase pipeline.
**Unaffected:** delete the fake ADK compat layer (D10's deletions), clean Sessions (D9), and build once at startup (D14). The candidate 02 to 05 findings stand. Grounding and scanning ports (candidate 02) become more important, because they are the tooling a Skill calls on.
- Branch: `refactor/phase-pipeline`, pushed to the Gitea remote; the PR is opened by a human.
## Problem
The five Phases are declared in seven places: `app/adk/agents.py`, `app/workflows/gcp_architecture_graph.py`, `app/nodes/__init__.py`, each node's `active_skills` bookkeeping, `app/card.py` (twice), and `workflow.yaml` (which already omits `source_discover`). They are run by three orchestrators: a LangGraph `StateGraph`, an "ADK" `SequentialAgent` inside a `LoopAgent`, and an "ADK" `Workflow`. None of these uses the real `google-adk` package. `HAS_NATIVE_ADK` is never read, so every "ADK" class is a local stub. The five agent classes are pass-throughs. Every node builds a skill prompt and discards it. The review loop reruns all five Phases without passing validation findings back. And passing a `session_id` merges a previous Execution's outputs into the next one.
## Goals
- One module owns Phase ordering, state merging, Phase contracts, the Review round, Skill binding and `active_skills`.
- One small interface: `async run(workflow_request) -> Execution`.
- Tests exercise the pipeline only through that interface.
- Adding or reordering a Phase is a one-row change to the phase table.
## Non-goals (other review candidates)
- Grounding and environment scan ports (candidate 02). Steps keep constructing `GCPEnvironmentScanner` and calling `get_mcp_client()`.
- The Execution store (candidate 03). `DatabaseManager` stays as it is, dual-dialect branches included.
- The Deliverable layout (candidate 04). The state-key → path mapping moves unchanged, and output paths in `workflow.yaml` are left alone.
- LangChain `@tool` pass-throughs in `app/tools/` (candidate 05). `validate` keeps calling `.invoke`.
- Typed Phase outputs (candidate 06). Outputs stay markdown strings.
- Making any Phase call an LLM. The interface gets ready for it; the step bodies keep their templates.
- A human approval gate, checkpointing or resume.
- Moving the transport to aiohttp A2A on :8000.
## Decisions
| # | Decision |
|---|---|
| D1 | LangGraph `StateGraph` orchestrates Phases. The ADK compat layer is deleted. Real ADK may be used *inside* a step later (ADR-0001). |
| D2 | Target direction: `discover`, `design` and `package` become LLM-driven; `source_discover` and `validate` stay deterministic. This work only readies the interface. |
| D3 | Stable: the `/generate` response shape and the Deliverable paths. `/card` is derived from the phase table. `workflow.yaml` is test-checked against it. Agent and log names may change. |
| D4 | No approval gate. There is a single `run`. |
| D5 | Review round: a conditional edge `validate → design` while validation fails and rounds < `MAX_REVIEW_ROUNDS`. Findings reach design as `review_feedback`. |
| D6 | Phase row: `Phase(name, skill, step, requires, produces)`. The pipeline enforces contracts, maintains `active_skills` and `current_phase`, and binds Skills by name. |
| D7 | Step: `async step(state, ctx: PhaseContext) -> dict`. `ctx` has `skill_prompt` and `model=None`. |
| D8 | The pipeline does no DB or file I/O. The caller records the Execution. |
| D9 | A Session groups Executions. Every Execution starts from clean state. |
| D10 | `app/adk/` is removed. `app/executions.py` records Executions for the route and the eval harness. |
| D11 | Module: `app/workflows/gcp_architecture_pipeline.py`. Steps stay in `app/nodes/`. The state schema stays in `app/states/`. |
| D12 | The phase table is a constructor argument, so tests inject a fake table. Old orchestrator tests are deleted. |
| D13 | The card is derived at import. `workflow.yaml` gains `source_discover`, and a test asserts its phase ids match. |
| D14 | The pipeline is built once in the Starlette lifespan and stored on `app.state.pipeline`. |
| D15 | When Review rounds run out with validation still failing, the pipeline packages anyway: `validation_passed=false`, status `completed_with_warnings`. |
| D16 | A contract breach raises `PhaseContractError`. The Execution is recorded `failed`, the route returns HTTP 500, and no Deliverables are written. |
| D17 | Blocking I/O in `source_discover` and `design` is offloaded with `asyncio.to_thread`. |
| D18 | Also: remove `google-adk` from `requirements.txt`, rewrite the README's ADK sections, delete the unreachable duplicate `return` in `routes.py`. |
## Design
### Module: `app/workflows/gcp_architecture_pipeline.py`
Replaces `app/workflows/gcp_architecture_graph.py`.
```python
@dataclass(frozen=True)
class PhaseContext:
skill_prompt: str
model: Any | None = None # D2: populated when a Phase becomes LLM-driven
Step = Callable[[Mapping[str, Any], PhaseContext], Awaitable[dict[str, Any]]]
@dataclass(frozen=True)
class Phase:
name: str # e.g. "design"
skill: str # Skill name, e.g. "architecture_design"
step: Step
requires: tuple[str, ...] # state keys that must be present (not None) before the step
produces: tuple[str, ...] # state keys the step must return
@dataclass(frozen=True)
class ReviewRound:
number: int # 1-based
passed: bool
findings: tuple[str, ...]
@dataclass(frozen=True)
class Execution:
execution_id: str
workflow_request: str
outputs: Mapping[str, Any] # final state values for every Phase's `produces`
validation_passed: bool
review_rounds: tuple[ReviewRound, ...]
active_skills: tuple[str, ...]
current_phase: str # last Phase run ("package" on success)
class PhaseContractError(Exception):
phase: str
missing: tuple[str, ...]
kind: Literal["requires", "produces"]
class PhasePipeline:
def __init__(self, phases: Sequence[Phase], skills: SkillLoader, max_review_rounds: int) -> None: ...
async def run(self, workflow_request: str) -> Execution: ...
PHASES: tuple[Phase, ...] = (...) # production table, below
```
**Construction invariants.** Each of these raises `ValueError` from `__init__`:
- phase names are not unique
- a Phase's `skill` is missing from the `SkillLoader`
- there is no Phase named `design` or `validate` (the Review round needs both)
The `StateGraph` is compiled once, in `__init__`.
**Graph.** Linear edges follow table order. The exception is one conditional edge leaving `validate`:
- **Loop back:** if `validation_passed` is false and the rounds so far are fewer than `max_review_rounds`, go to `design`.
- **Otherwise:** continue to the Phase after `validate`.
**Per-Phase node wrapper.** The pipeline generates one per row, and it does the following in order:
1. **Check inputs.** Every `requires` key must be present and not None in state. Otherwise raise `PhaseContractError(kind="requires")`.
2. **Run the step.** Build `PhaseContext(skill_prompt=<that Skill formatted for prompt>)` and await `step(read-only state view, ctx)`.
3. **Check outputs.** Every `produces` key must be in the returned dict. Otherwise raise `PhaseContractError(kind="produces")`.
4. **Merge only declared keys.** Keys the step returns that aren't in `produces` are dropped, with a warning logged.
5. **Update bookkeeping.**
- Set `current_phase` to the Phase name.
- Append the Skill to `active_skills` if it isn't already there.
- For `design`: increment `review_round`.
- For `validate`: append a `ReviewRound(review_round, validation_passed, errors)` to `review_rounds`. On failure, set `review_feedback = errors`.
**Key presence.** A key is present if it is not None. `False`, `""` and `[]` all count as present.
**`run`.**
- **Initial state:** `{execution_id: uuid4, workflow_request, active_skills: [], review_round: 0, review_rounds: [], review_feedback: []}`. Nothing is carried over from a Session (D9).
- **Execution:** `ainvoke` the compiled graph, then build the `Execution`.
- **Errors:** `PhaseContractError` propagates to the caller.
### State: `app/states/state.py`
`GCPArchitectureState` changes:
- **Added:** `execution_id: str`, `review_round: int`, `review_rounds: list[ReviewRound]`, `review_feedback: list[str]`.
- **Removed:** `target_dir`. Nothing reads it; `/generate` still accepts the field and ignores it, as it effectively does today.
### Production phase table
| Phase | Skill | requires | produces |
|---|---|---|---|
| `source_discover` | `source_discovery` | `workflow_request` | `source_discovery_doc`, `source_mermaid_diagram` |
| `discover` | `requirements_discovery` | `workflow_request` | `requirements_doc`, `product_selection_deferred` |
| `design` | `architecture_design` | `requirements_doc` | `architecture_doc`, `mermaid_diagram`, `terraform_code`, `product_selection_deferred` |
| `validate` | `validation_rules` | `mermaid_diagram`, `terraform_code` | `validation_results`, `validation_passed`, `errors` |
| `package` | `packaging_guide` | `source_discovery_doc`, `source_mermaid_diagram`, `requirements_doc`, `architecture_doc`, `mermaid_diagram`, `terraform_code`, `validation_results` | `solution_guide`, `status_summary` |
`design` doesn't read `requirements_doc` today. The table declares the dependency the Phase is meant to have (see `workflow.yaml`). `design` may also read `review_feedback`, which is always present.
### Steps: `app/nodes/*.py`
- **New signature:** each `*_node(state, skill_loader)` becomes `async def *_step(state, ctx) -> dict` (D7). Bodies keep their current templates.
- **Removed from steps:** each step returns only its `produces` keys. The `skill_loader.format_skills_for_prompt(...)` call and the `active_skills`/`current_phase` bookkeeping are deleted.
- **Blocking I/O (D17):**
- `source_discover`: `scan_result = await asyncio.to_thread(scanner.scan_environment)`. The scanner is also constructed inside the thread, because its `__init__` can shell out to `gcloud`.
- `design`: `await asyncio.to_thread(mcp_client.search_documents, ...)`.
- **Skills:** `SkillLoader` gains `format_skill_for_prompt(name) -> str`, which gives the single-Skill form of `format_skills_for_prompt`.
### Recording: `app/executions.py`
A plain module that replaces `ADKAgentRunner`, `PostgresSessionService`, `PostgresArtifactRepository` and `ADKEvaluator`'s persistence. It uses `get_db_manager()` for now (candidate 03 replaces that).
- `record(execution, session_id, base_dir) -> None`:
- Writes Deliverables using the state-key → path mapping moved verbatim from `app/adk/artifacts.py`. That covers current paths, per-execution paths and `docs/` mirrors.
- Saves one `adk_artifacts` row per written file.
- Saves the session row (`agent_name = settings.AGENT_NAME`, `state_data` = the Execution as JSON).
- Saves a `workflow_executions` row with status `completed` or `failed` from `validation_passed`, `loop_count = len(review_rounds)`, and the results JSON as today.
- Writes one `orchestrator_review_logs` row per `ReviewRound`: `reviewer_agent="validate"`, `review_status` `APPROVED` or `NEEDS_REVISION`, `feedback` = the joined findings.
- `record_failure(execution_id, session_id, workflow_request, error: PhaseContractError) -> None`: saves a `workflow_executions` row with status `failed`, `current_phase = error.phase` and the error details in the results JSON. It writes no Deliverables.
- `to_generate_response(execution, session_id) -> dict` builds the stable `/generate` shape:
- `execution_id`, `session_id`, `validation_passed`, `current_phase`, `active_skills`
- `status`: `success` or `completed_with_warnings`
- `total_loop_iterations`: `len(review_rounds)`
- `artifacts`: the same 8 keys as today
### HTTP: `app/agent.py`, `app/workflows/routes.py`
- **Lifespan:** runs `get_db_manager()` as today, then `app.state.pipeline = PhasePipeline(PHASES, loaded SkillLoader, settings.MAX_REVIEW_ROUNDS)` (D14).
- **`POST /generate`:** `await request.app.state.pipeline.run(workflow_request)`, then `executions.record(...)`, then return `to_generate_response(...)`.
- On `PhaseContractError`: call `executions.record_failure(...)` and return HTTP 500 with `{"error": str(e), "phase": e.phase, "missing": [...]}`.
- A missing or invalid `session_id` in the body gets a new uuid, as today.
- **`GET /sessions/{id}`:** reads through `get_db_manager().get_session`. The response shape is unchanged, but `agent_name` becomes `gcp_solution_architecture_agent`.
- **`GET /health`:** drops `"adk_framework"`.
- **`POST /validate`:** unchanged. Delete the unreachable duplicate `return`.
### Card and manifest
- `app/card.py`: `capabilities.phases = [p.name for p in PHASES]` and `capabilities.local_skills = [p.skill for p in PHASES]`. All other fields are unchanged.
- `workflow.yaml`: add a `source_discover` phase entry before `discover`. Renumbering `step` is left to the implementer, as long as order matches `PHASES`.
### Settings: `app/config.py`, `.env.example`
- `MAX_REVIEW_ROUNDS: int = 5`, which also reads the legacy env name `ADK_MAX_LOOP_ITERATIONS` (pydantic `AliasChoices`).
- Delete `ADK_MAX_LOOP_ITERATIONS` and `ADK_ENABLE_ARTIFACT_STORE` (the latter is never read).
- Update `.env.example` to match.
### Eval: `eval/eval_harness.py`
- `EvalHarness.run_eval_suite` becomes `async`. `EvalHarness` builds a `PhasePipeline`, awaits `run(case["workflow_request"])`, scores `execution.outputs` with `eval/metrics.evaluate_case_run`, and saves with `get_db_manager().save_evaluation`.
- `main()` uses `asyncio.run`.
- This removes the `app → eval` import that `app/adk/evaluation.py` created.
### Deleted
- `app/adk/` (all 9 files)
- `app/workflows/gcp_architecture_graph.py`
- `tests/test_graph_workflow.py`
- `google-adk` from `requirements.txt`
## Behaviour changes (call out in the PR)
1. A `session_id` no longer seeds an Execution with a previous Execution's state.
2. A failing validation now re-runs only `design → validate`, not all five Phases.
3. A Phase contract breach returns HTTP 500 and records a `failed` Execution. Before, the pipeline silently continued with empty strings.
4. `/sessions/{id}` returns `agent_name: "gcp_solution_architecture_agent"`, not `OrchestratorLoopAgent`.
5. `/health` no longer includes `adk_framework`.
6. `total_loop_iterations` counts Review rounds, not whole-pipeline passes. Its value is `1` on the happy path, the same as today.
7. `workflow.yaml` lists `source_discover`.
## Testing
Tests go through `PhasePipeline.run()` and the HTTP interface only, using `pytest-asyncio`.
**New: `tests/test_phase_pipeline.py`**
These use a fake phase table of tiny async steps and a stub `SkillLoader` fixture (or a tmp skills dir):
- **Phase order:** Phases run in table order, and `current_phase` ends on the last Phase.
- **Merging:** only declared `produces` keys are merged, and extra keys are dropped.
- **Requires check:** a missing `requires` key raises `PhaseContractError(kind="requires", phase=...)`.
- **Produces check:** a step that omits a `produces` key raises `PhaseContractError(kind="produces", phase=...)`.
- **Presence:** `False` and `""` count as present.
- **Review round, passes on round 2:** `validate` fails once and then passes. `design` runs twice and receives `review_feedback` on round 2; `review_rounds` has 2 entries and `validation_passed` is true.
- **Review round, never passes:** `validate` always fails. Rounds stop at `max_review_rounds`, the Phase after `validate` still runs, and `validation_passed` is false.
- **Skills:** `ctx.skill_prompt` holds the bound Skill's content, and `active_skills` lists each Skill once, even across Review rounds.
- **Clean runs:** two `run()` calls on one pipeline share no state.
- **Construction:** duplicate names, an unknown Skill, or a missing `design`/`validate` each raise `ValueError`.
**Also new:**
- **End to end:** `run()` with the real `PHASES` and `app/skills`. Every `produces` key is present, `validation_passed` is true, there are 5 `active_skills` and 1 review round.
- **Manifest check:** the phase ids in `workflow.yaml` equal `[p.name for p in PHASES]`, and `/card` phases and skills equal the table.
**Changed**
- `tests/test_agent_api.py`: use `with TestClient(app) as client`, because the lifespan must run so `app.state.pipeline` exists. Existing assertions stay and guard the stable `/generate` contract. Add one test for the 500 response on a contract breach, with a pipeline built from a breaching fake table swapped onto `app.state`.
- `tests/test_adk_architecture.py`:
- **Delete:** `test_adk_tools`, `test_adk_postgres_session_service`, `test_adk_orchestrator_loop_agent`, `test_adk_workflow_composition`, `test_adk_agent_runner`, `test_adk_evaluator`.
- **Keep:** `test_database_manager_operations`, moved to `tests/test_database.py`.
- **Move:** the session endpoint test into `test_agent_api.py`, asserting the new `agent_name`.
- Then delete the file.
- `tests/test_mcp_developer_knowledge.py`: delete `test_adk_mcp_function_tools` and its `app.adk.tools` import.
- `eval/test_eval_harness.py`: `test_eval_harness_benchmark` awaits the now-async `EvalHarness.run_eval_suite()` (`@pytest.mark.asyncio`), and its assertions are unchanged. `test_skill_optimizer_recommendations` is untouched.
**Known issue, left out of scope:** `/generate` tests write Deliverables into the working tree and the local SQLite file (candidates 03 and 04).
## Implementation increments
These are ordered. Each one leaves the suite green and is one or more commits on `refactor/phase-pipeline`. They are called *increments* to avoid clashing with the glossary's **Phase**.
1. **Baseline environment.**
- Create a venv and install `requirements.txt` and `requirements-dev.txt`.
- Run the current suite and record the baseline pass/fail list in this spec's `## Comments`. No Python environment exists on the dev machine yet.
- Create the branch.
2. **Pipeline core, test-first.**
- Add `Phase`, `PhaseContext`, `ReviewRound`, `Execution`, `PhaseContractError`, `PhasePipeline` and `SkillLoader.format_skill_for_prompt`.
- Write every fake-table test in `test_phase_pipeline.py` first.
- Production code is untouched apart from these additions.
3. **Port the steps and the table.**
- Convert the five nodes to async steps (including D17), define `PHASES`, extend `GCPArchitectureState` and add the end-to-end test.
- Delete `gcp_architecture_graph.py` and `test_graph_workflow.py`, and update `app/workflows/__init__.py` and `app/nodes/__init__.py` exports.
- The `app/adk/` agents still call the old node names at this point, so either keep thin old-signature shims until increment 5 or do increments 3 and 5 together. Prefer shims, deleted in 5.
4. **Recording and HTTP wiring.**
- Add `app/executions.py`, the lifespan pipeline, the `/generate` success and 500 paths, `/sessions` via `DatabaseManager`, and `/health`.
- Rename settings, with the alias, and update `.env.example`.
- Update `test_agent_api.py` (lifespan client, 500 test, session endpoint test).
5. **Remove ADK.**
- Move the eval harness onto the pipeline.
- Delete `app/adk/`, the old-signature shims, the ADK tests and `google-adk` from `requirements.txt`.
- Move the DB test to `tests/test_database.py`.
6. **Card and manifest.** Derive the card from `PHASES`, add `source_discover` to `workflow.yaml`, and add the manifest consistency test.
7. **Docs and cleanup.**
- Rewrite the README's ADK sections around the Phase pipeline.
- Delete the duplicate `return` in `routes.py`.
- Run a final full suite plus `ruff` and `mypy` on the touched modules.
- Push the branch.
## Acceptance
- [ ] `rg -n "app\.adk|google\.adk|OrchestratorLoopAgent|compat" app eval tests` returns nothing.
- [ ] The phase list appears only in `PHASES`, and `workflow.yaml` is test-checked against it.
- [ ] Every test listed under Testing exists and passes. The rest of the baseline suite is no worse than increment 1's record.
- [ ] The `/generate` response keys match today's exactly: `execution_id`, `session_id`, `status`, `validation_passed`, `total_loop_iterations`, `current_phase`, `artifacts` (8 keys) and `active_skills`.
- [ ] The Deliverable paths written by `/generate` are unchanged.
- [ ] The README no longer describes ADK orchestration, and ADR-0001 is linked.

470
.scratch/solution-architect-agent/spec.md Normal file
View File

@@ -0,0 +1,470 @@
# Spec: Solution architect agent
A portable, hierarchical ReAct agent that replaces the upstream [`google-cloud-solution-architecture`](https://github.com/google/skills/blob/main/skills/cloud/google-cloud-solution-architecture/SKILL.md) skill. An Orchestrator reasons over a code-declared dependency graph of Deliverables and delegates to Specialists. All agents run vendored Agent Skills through a framework's own skill runtime. Users grant Approvals at Phase boundaries.
- Glossary: [`CONTEXT.md`](../../CONTEXT.md). Use its terms: Execution, Session, Phase, Approval, Revision, Orchestrator, Specialist, Skill, Cloud provider, Deliverable, Dependency, Solution guide.
- Decisions: [ADR-0002](../../docs/adr/0002-skills-are-the-portable-unit-of-behaviour.md) (Skills-first, hybrid, hierarchical and ReAct, vendoring, Cloud provider as configuration) and [ADR-0003](../../docs/adr/0003-langgraph-holds-state-adk-agents-reason.md) (LangGraph state with ADK agents).
- Supersedes: [`.scratch/phase-pipeline/spec.md`](../phase-pipeline/spec.md). Its decision log (D1 to D46) records how this design was reached.
- Branch: `feat/solution-architect-agent`, pushed to the Gitea remote. A human opens the PR.
## Problem
The current agent is a linear chain of five template functions. They ignore the Workflow request (design always picks Cloud Run and Pub/Sub). They run under a fake "ADK" layer that never touches `google-adk`. The skill prompts they build are thrown away. The agent can't ask the user anything, can't take an Approval, can't run the upstream Skill, and can't target another framework or Cloud provider without a rewrite.
## Goals
- **Upstream parity.** It runs the upstream Skill's workflow: requirements discovery with strict separation from solutioning, grounded product selection with alternatives, diagram, description, WAF-pillar design recommendations, IaC deployment guidance, dry-run validation, and packaging into the upstream template.
- **Hierarchical and ReAct.** Work follows Dependencies, not a fixed order, and independent work runs in parallel.
- **Interactive by default**, with code-enforced, recorded Approvals at Phase boundaries. There is also a pre-approved mode for batch and evaluation runs.
- **Discovery through the Cloud provider's own remote MCP servers.** Current state is discovered through remote MCP servers hosted by the Cloud provider, such as Google Cloud's Resource Manager, Asset Inventory, Compute and GKE servers. Grounding uses the provider's knowledge MCP server. No hand-written cloud REST scanners.
- **Portable.**
- Behaviour lives in Skills and framework-neutral role files.
- Framework types stay behind one agent-runtime interface.
- Everything specific to a Cloud provider lives in a provider bundle.
- **Deterministic tests** check every rule the code owns, without a model.
## Non-goals (v1)
- **Runtime validation.** Upstream Task 3.2, verifying after the user has deployed, is out of scope.
- **Changing cloud resources.** No `apply` or deploy tool, and ADK's `run_skill_script` is off.
- **A second Cloud provider** or a second agent-runtime implementation (Deep Agents). The design leaves room for both; neither is built.
- **Architecture review candidates 02 to 06**, beyond what this spec needs: tools get tier wrappers, not full ports; `DatabaseManager` is reused as is; no typed Deliverable records.
- **Human approval inside a Phase enforced by code.** That stays conversational and driven by the Skill (see [Risks](#risks)).
- **Acting as the end user in their cloud (OAuth).** Remote MCP calls use the agent's own identity (a service account or Application Default Credentials) in v1.
- **Routing MCP calls through the platform's MCP gateway** (`mcp_gateway_client.py` in the base agents). Remote MCP servers are called directly (D46). If a gateway is needed later, it becomes a new `mcp_auth.kind: gateway` implementation in `app/tools/mcp.py`, and nothing else changes.
## Decisions
These are carried from the decision log. The numbers refer to it.
| Area | Decision |
|---|---|
| Sequencing (D19, D23) | **Hybrid.** Code owns Dependencies, Phase boundaries and Approvals, and the Skill owns the work inside a Deliverable. The agent is hierarchical and ReAct, not linear. |
| Engine (D24) | **LangGraph + ADK.** A LangGraph `StateGraph` holds Execution state, checkpoints and `interrupt()`. The Orchestrator and Specialists are ADK `LlmAgent`s with `SkillToolset`, behind an `AgentRuntime` interface. |
| Phases (D25) | **Upstream's 4 Phases**, as groupings of Deliverables. Discovery of current state is done through tools (remote MCP servers, D45). |
| Approvals (D26, D32) | Code enforces Approval at Phase boundaries. A Dependency inside a Phase is met on *submitted*, and one across Phases only on Phase *approved*. Every user question is an `interrupt()`. |
| Completion (D27) | Agents finish by calling a submit tool that is checked against the Deliverable's contract. |
| Transport (D28) | **A2A** for interactive Executions. `/generate` is kept for pre-approved ones. |
| Hierarchy (D29) | **An Orchestrator over one Specialist per Deliverable.** The design advisor fans out to 6 pillar advisors. |
| Dependency graph (D30) | **Declared in code.** The ReAct Orchestrator picks work through tools that refuse work whose Dependencies aren't met. |
| Budgets (D31) | **Per-agent ReAct step budgets.** Hitting one hands control to the user. |
| Voice (D33) | **Only the Orchestrator talks to the user.** Specialists ask through `needs_input`. |
| Specialist memory (D34) | **Stateless per delegation.** Only the LangGraph checkpoint persists. |
| Revision (D35) | **Reopening a Deliverable** returns it and everything that depends on it to draft and withdraws affected Approvals. Revisions are capped, and past the cap the user decides. |
| Models (D36) | **Configurable per role via ADK LiteLLM**, with a Gemini default. |
| Dependency graph content (D38) | See [Dependency graph](#dependency-graph). Runtime validation is out of scope. |
| Skill slicing (D39) | **A shared `SkillToolset` plus `agents/<role>.md`.** Pillar advisors are scoped to their own WAF Skill. |
| Vendoring (D21, D40) | **Upstream solution-architecture + 6 WAF Skills**, unmodified, pinned to `81a31a6`, in `skills/vendor/google/`. |
| Tool tiers (D41) | **General tools** (local checks, public documentation) run freely. **Reading live cloud resources** needs permission once per Execution. **Changing cloud resources** is never exposed. |
| Discovery (D45) | **The Cloud provider's own remote MCP servers**, declared in the provider bundle, provide discovery and grounding. Every tool that reads the user's cloud is `cloud_read`. Only tools on an explicit read-only allowlist are exposed. `GCPEnvironmentScanner` is retired. |
| MCP routing (D46) | **Call remote MCP servers directly**, with no platform MCP gateway for now. |
| Outputs (D42) | **Deliverables live in state and the database.** Files go to `deliverables/executions/{id}/` only after Phase 4 Approval. |
| Tests (D43) | **A fake `AgentRuntime`** for deterministic tests. The real-model evaluation runs pre-approved with scripted answers. |
| Migration (D44) | **Build alongside the current code.** One cleanup increment follows once the evaluation passes. |
| Carried over (D9, D14) | **Every Execution starts clean**, and a Session only groups Executions. The Execution graph and agent runtime are built once at startup. |
## Design
### Module layout
New code only. The current modules stay untouched until [increment 9](#build-increments).
```
agents/ # framework-neutral role files (one per role)
orchestrator.md
requirements-analyst.md decomposer.md product-selector.md diagrammer.md
describer.md design-advisor.md pillar-advisor.md iac-author.md
validator.md packager.md
providers/
gcp.yaml # Cloud provider bundle
skills/
vendor/google/ # unmodified upstream Skills + VENDOR.md + LICENSE
google-cloud-solution-architecture/
google-cloud-waf-{security,reliability,cost-optimization,operational-excellence,performance-optimization,sustainability}/
cloud-discovery/SKILL.md # our tooling Skills (Agent Skills spec compliant)
iac-validation/SKILL.md
app/
execution/
deliverables.py # DeliverableKind, PhaseId, DEPENDENCY_GRAPH, contracts
ledger.py # Ledger: Deliverable/Phase states, acceptance, Revision (pure, no I/O)
graph.py # LangGraph StateGraph: orchestrator turn, Specialist fan-out, interrupts
tools.py # Orchestrator and Specialist execution tools bound to the Ledger
state.py # ExecutionState TypedDict
recorder.py # records submissions, Approvals, Revisions, final files
agents/
runtime.py # AgentRuntime protocol, RoleSpec, AgentTask, Outcome types
roles.py # loads agents/*.md into RoleSpec
adk_runtime.py # ADK LlmAgent + SkillToolset + LiteLlm implementation
fake_runtime.py # scripted implementation for tests (under app/ so eval can reuse it)
tools/
general/ # mermaid check, terraform fmt/validate (local)
cloud_read/ # terraform plan (local CLI, reads live state)
mcp.py # McpServerSpec, read-only allowlist, tier + permission metadata
providers.py # loads providers/<name>.yaml
a2a/
server.py # aiohttp A2A server on :8000, agent_card.json
executor.py # maps A2A tasks <-> Executions and interrupts
scripts/sync-vendored-skills.sh
```
### Deliverables and the dependency graph <a id="dependency-graph"></a>
`app/execution/deliverables.py` declares the graph as data. It is the only place Phases, Deliverables and Dependencies are listed.
| Phase | Deliverable kind | Specialist role | Depends on | Tools (tier) |
|---|---|---|---|---|
| 1 `requirements_discovery` | `requirements` | `requirements-analyst` | (none) | discovery MCP servers (cloud_read) |
| 1 | `technical_decomposition` | `decomposer` | `requirements` | (none) |
| 2 `solution_architecture` | `product_selection` | `product-selector` | `technical_decomposition` | knowledge MCP (general) |
| 2 | `architecture_diagram` | `diagrammer` | `product_selection` | mermaid check (general) |
| 2 | `architecture_description` | `describer` | `architecture_diagram`, `product_selection` | knowledge MCP (general) |
| 2 | `design_recommendations` | `design-advisor` → 6 × `pillar-advisor` | `product_selection`, `requirements` | knowledge MCP (general) |
| 2 | `deployment_guidance` | `iac-author` | `product_selection`, `design_recommendations` | terraform fmt/validate (general) |
| 3 `solution_validation` | `validation_results` | `validator` | `deployment_guidance` | terraform validate (general), terraform plan and policy MCP servers: IAM, Network Intelligence (cloud_read) |
| 4 `packaging` | `solution_guide` | `packager` | every Phase 3 Deliverable | (none) |
**Deliverable contract.** Each kind declares its required content in `deliverables.py`:
- `sections`: required markdown headings.
- `attachments`: named code blobs, for example `deployment_guidance` requires `terraform/main.tf` and `terraform/variables.tf`.
`submit_deliverable` checks the contract. A failing submission returns what's missing to the Specialist, and nothing is recorded.
### The Ledger: acceptance and Revision rules
`app/execution/ledger.py` is a pure module. It holds every rule code owns, and the tests exercise it heavily.
- **Deliverable states:** `pending → in_progress → submitted`. Only a Revision goes backwards.
- **Phase states:** `open → awaiting_approval → approved`. A Phase becomes `awaiting_approval` when the Orchestrator calls `request_approval` with all its Deliverables `submitted`.
- **When a Dependency is met:**
- **Same Phase:** the Dependency is `submitted`.
- **Across Phases:** the Dependency's Phase is `approved`.
- **Ready work:** Deliverables that are `pending` and have every Dependency met.
- **`reopen(kind, reason)`:**
- The Deliverable goes to `pending` with `reason` appended to its feedback.
- Everything that depends on it, directly or not, goes to `pending`.
- Every affected Phase that was `approved` or `awaiting_approval` goes back to `open`.
- The Deliverable's `revisions` count goes up. At the cap (`max_revisions`, default 3) `reopen` refuses and returns an escalation the Orchestrator must put to the user.
- **Pre-approved mode:** `request_approval` grants immediately and records `approved_by = "pre-approved"`.
- **Records:** every transition appends an event (`submitted`, `approval_requested`, `approved`, `changes_requested`, `reopened`, `escalated`) for the recorder.
### Execution graph (LangGraph)
`app/execution/graph.py` compiles once at startup with the configured checkpointer: `SqliteSaver` locally and in tests, `PostgresSaver` in production. The Execution id is the LangGraph `thread_id`.
**State (`ExecutionState`):**
- `execution_id`, `session_id`, `workflow_request`, `mode: "interactive" | "pre_approved"`, `allow_cloud_read: bool | None`
- `ledger` (serialisable), `deliverables: {kind: {content, attachments, feedback[], revisions}}`
- `conversation[]` (user ↔ Orchestrator turns only), `pending_questions[]`, `pending_delegations[]`
- `budget_exhausted: {role, summary} | None`
**Nodes and edges:**
```
START → orchestrator_turn
orchestrator_turn ──(delegations)──────────> specialist_task × N (LangGraph Send, parallel)
orchestrator_turn ──(questions/approval)───> await_user (interrupt)
orchestrator_turn ──(finish)───────────────> finalize → END
specialist_task ───────────────────────────> orchestrator_turn (outcomes merged into ledger)
await_user ────────────────────────────────> orchestrator_turn (resume value = user reply / approval decision)
```
- **`orchestrator_turn`:** runs one Orchestrator ReAct turn through `AgentRuntime`, with Orchestrator tools bound to a working copy of the Ledger. The turn ends when the agent does one of these, and the node routes on it:
- calls `delegate`, `ask_user`, `request_approval` or `finish`
- runs out of budget
- **`specialist_task`:** runs one Specialist delegation. Its task holds the role, the Deliverable kind, the Dependency contents, feedback, and the answers to its earlier `needs_input` questions. The outcome is one of:
- `Submitted(content, attachments)`, applied to the Ledger
- `NeedsInput(questions)`, added to `pending_questions` with the Deliverable back to `pending`
- `BudgetExhausted(summary)`
- **`await_user`:** `interrupt({"questions": [...]} | {"approval": {"phase": ..., "summary": ...}} | {"permission": "cloud_read"} | {"escalation": ...})`. In pre-approved mode it doesn't pause:
- **Approvals:** granted automatically.
- **`cloud_read` permission:** taken from the request.
- **Questions:** answered with `"No user is available. State your assumption, record it in the Deliverable under 'Assumptions', and proceed."`
- **Escalations:** end the Execution as `completed_with_warnings`.
- **`finalize`:** runs after Phase 4 is `approved`. It writes files to `deliverables/executions/{execution_id}/`: the Solution guide, every Deliverable as markdown, and attachments under `terraform/`. It then records completion.
**Orchestrator tools** (`app/execution/tools.py`):
| Tool | Effect |
|---|---|
| `ready_work()` | Lists ready Deliverables and Phases ready for Approval. |
| `delegate(kinds: list[str])` | Refuses any kind that isn't ready and sets it `in_progress`. Ends the turn and fans out. |
| `ask_user(questions: list[str])` | Combines Specialist questions with its own. Ends the turn and interrupts. |
| `request_approval(phase: str)` | Refuses unless every Deliverable in the Phase is submitted. Ends the turn and interrupts. |
| `reopen(kind: str, reason: str)` | Applies the Revision rule, or returns an escalation. |
| `read_deliverable(kind: str)` | Returns the current content, so the Orchestrator can summarise it for Approval. |
| `finish()` | Refuses unless Phase 4 is approved. |
**Specialist tools:** `submit_deliverable(content, attachments)` (checks the contract), `needs_input(questions)`, the `SkillToolset`, and the role's tier tools. Cloud-read tools check `allow_cloud_read`. If it's `None`, the tool returns `NeedsInput(["May I read live resources in your Google Cloud project to …?"])` so the Orchestrator asks once. If it's `False`, it returns a refusal the agent must work around.
**Design advisor fan-out.** The `design-advisor` role runs a nested fan-out itself: it sends one task to `pillar-advisor` for each WAF pillar, in parallel, through `AgentRuntime`, and combines the results into its submission. Pillar results are not Deliverables and don't appear in the Ledger.
### Agent runtime interface
`app/agents/runtime.py`:
```python
class AgentRuntime(Protocol):
async def run(self, role: RoleSpec, task: AgentTask, tools: Sequence[ToolSpec]) -> Outcome: ...
@dataclass(frozen=True)
class RoleSpec: # parsed from agents/<role>.md
name: str
instructions: str # markdown body
skills: tuple[str, ...] # Skill names from the provider bundle
tool_tiers: tuple[str, ...] # subset of ("general", "cloud_read")
mcp_servers: tuple[str, ...] # server ids from the provider bundle this role may use
model: str | None # overrides provider default
budget: int # max tool calls per run
Outcome = Submitted | NeedsInput | BudgetExhausted | TurnEnded # TurnEnded carries the Orchestrator's ending tool call
```
- **`ToolSpec`:** framework-neutral (name, description, JSON schema, async callable). `adk_runtime.py` builds each agent from these pieces:
- `ToolSpec` → ADK `FunctionTool`.
- The role's MCP servers → ADK `McpToolset` over streamable HTTP, with `tool_filter` set to the server's read-only allowlist.
- `SkillToolset(skills=[load_skill_from_dir(...)])` with only the role's Skills.
- `model` wrapped in `LiteLlm`.
- **Per run:** a fresh `InMemorySessionService` (D34).
- **Callbacks:** a `before_tool_callback` counts tool calls against `budget` and enforces the `cloud_read` permission for *every* tool in that tier, MCP tools included.
- **No framework types** cross this interface.
**Role file format** (`agents/<role>.md`):
```markdown
---
role: diagrammer
deliverable: architecture_diagram
upstream_task: "Phase 2, Task 2.2: Generate an architecture diagram"
skills: [google-cloud-solution-architecture]
tool_tiers: [general]
mcp_servers: []
budget: 15
---
You produce the architecture diagram Deliverable. Load the `google-cloud-solution-architecture` Skill and follow its Task 2.2 ...
Submit with `submit_deliverable` once the diagram passes the Mermaid check and the user has confirmed it through `needs_input`.
```
`pillar-advisor.md` is parameterised by pillar, and its `skills` value is resolved per task to the one matching `google-cloud-waf-<pillar>`.
### Cloud provider bundle
`providers/gcp.yaml`, loaded by `app/providers.py`:
```yaml
name: gcp
skills_dirs: [skills/vendor/google, skills]
output_template: skills/vendor/google/google-cloud-solution-architecture/assets/output-template.md
models:
default: gemini/gemini-2.5-pro
roles: { pillar-advisor: gemini/gemini-2.5-flash, diagrammer: gemini/gemini-2.5-flash }
tools:
general: [mermaid_check, terraform_fmt, terraform_validate]
cloud_read: [terraform_plan]
mcp_auth:
kind: google_adc # service account / ADC bearer token; sends X-Goog-User-Project
mcp_servers: # remote MCP servers hosted by the Cloud provider (streamable HTTP)
developer_knowledge:
url: https://developerknowledge.googleapis.com/mcp
tier: general # public documentation, no customer data
allow: [search_documents, get_documents, answer_query]
resource_manager: { url: https://cloudresourcemanager.googleapis.com/mcp, tier: cloud_read, allow: [<read-only tools>] }
asset_inventory: { url: https://cloudasset.googleapis.com/mcp, tier: cloud_read, allow: [...], preview: true }
compute: { url: https://compute.googleapis.com/mcp, tier: cloud_read, allow: [...] }
gke: { url: https://container.googleapis.com/mcp, tier: cloud_read, allow: [...] }
cloud_run: { url: https://run.googleapis.com/mcp, tier: cloud_read, allow: [...] }
cloud_sql: { url: https://sqladmin.googleapis.com/mcp, tier: cloud_read, allow: [...] }
storage: { url: https://storage.googleapis.com/storage/mcp, tier: cloud_read, allow: [...] }
pubsub: { url: https://pubsub.googleapis.com/mcp, tier: cloud_read, allow: [...] }
iam: { url: https://iam.googleapis.com/mcp, tier: cloud_read, allow: [...] }
monitoring: { url: https://monitoring.googleapis.com/mcp, tier: cloud_read, allow: [...] }
network_intelligence: { url: https://networkmanagement.googleapis.com/mcp, tier: cloud_read, allow: [...], preview: true }
role_mcp_servers:
requirements-analyst: [resource_manager, asset_inventory, compute, gke, cloud_run, cloud_sql, storage, pubsub, iam, monitoring]
product-selector: [developer_knowledge]
describer: [developer_knowledge]
design-advisor: [developer_knowledge]
pillar-advisor: [developer_knowledge]
validator: [iam, network_intelligence]
```
**Loading the bundle.**
- Settings select the bundle with `CLOUD_PROVIDER=gcp`. Core modules don't name a provider.
- `role_mcp_servers` overrides the `mcp_servers` list in role files, so role files stay neutral across providers.
- The `allow` lists are written once in increment 6: run MCP `tools/list` on each server and include only tools whose `readOnlyHint` annotation is true *and* whose names are reviewed as read-only.
- Servers marked `preview: true` can be switched off with `MCP_ENABLE_PREVIEW=false`.
**Other Cloud providers** reuse the same shape:
- **AWS:** the managed, generally available AWS MCP Server.
- **Azure:** Azure MCP Server 2.0 runs as a *self-hosted* remote server, so its `url` points at the team's own deployment.
### Tools
- **General, local** (`app/tools/general/`):
- `mermaid_check`: moved from `validation_tools.validate_mermaid_diagram`.
- `terraform_fmt` and `terraform_validate`: run the `terraform` CLI in a temp dir with `-backend=false`. There are no credentials, and the tool fails clearly if `terraform` is missing.
- **Cloud read, local** (`app/tools/cloud_read/`): `terraform_plan` needs ADC and `-lock=false`, and never runs `apply`.
- **Remote MCP servers** (`app/tools/mcp.py`): the Cloud provider's own hosted servers, declared in the provider bundle and used for both discovery and grounding. They replace `GCPEnvironmentScanner` (hand-written REST calls) and the hand-rolled JSON-RPC `DeveloperKnowledgeMCPClient`, including its offline fallback knowledge base and the `data`/`documents` bug from review candidate 02. Both are deleted in increment 9.
- `McpServerSpec(id, url, tier, allow, preview)` is framework-neutral. The ADK runtime maps each one to `McpToolset(StreamableHTTPConnectionParams(url, headers=auth), tool_filter=allow)`.
- **Read-only in depth.** Tools outside `allow` are never offered to any agent. At startup, a server whose `tools/list` reports an allowed tool with `readOnlyHint` not true fails to load, loudly. Operators should also run the agent's identity with only viewer roles, plus Google Cloud's control that blocks read-write MCP tool use and an IAM deny policy (`docs/operations/mcp-access.md`, written in increment 6).
- **Auth:** `google_adc` gets a bearer token from Application Default Credentials (`google.auth.default`, refreshed per call) and sends `X-Goog-User-Project`. Follows `google-adk-base-agent/app/google_mcp_client.py`, without the `gcloud` subprocess fallback.
- **Failure:** an unreachable, disabled or unauthorised server returns a structured tool error such as `"compute MCP unavailable: API not enabled in project X"`. The agent records the gap under 'Assumptions / discovery gaps' and doesn't fail the Execution.
- **Our tooling Skills** (`skills/cloud-discovery`, `skills/iac-validation`): short Agent Skills-compliant guides for agents.
- `cloud-discovery`: which discovery servers answer which current-state questions (inventory first through Asset Inventory and Resource Manager, then per-product detail), and how to summarise current state.
- `iac-validation`: when and how to run the Terraform checks.
### Interaction
**A2A** (`app/a2a/`), using `a2a-sdk>=0.3` on `aiohttp` :8000, with `agent_card.json` and `/.well-known/agent.json`:
- **Task ↔ Execution:** an A2A task is an Execution (`task.id` = `execution_id`), and `context_id` is the Session.
- **Status updates:** `working` while nodes run.
- **`input-required`** at every `interrupt()`. The interrupt payload goes out as a `DataPart`, with a text rendering for plain clients.
- **User reply:** a text part becomes an answer to pending questions. A `DataPart` with `{"approval": {"phase", "decision": "approve" | "changes", "feedback"}}` or `{"permission": {"cloud_read": bool}}` resumes an Approval or permission interrupt. **Approvals are never taken from free text**, which keeps them explicit and recorded.
- **Final state:** `completed` after `finalize`, and `failed` on unhandled errors.
**`POST /generate`** (existing Starlette app, pre-approved mode):
- **Request:** `{request, session_id?, allow_cloud_read?: false}`.
- **Response:** today's keys (`execution_id`, `session_id`, `status`, `validation_passed`, `total_loop_iterations`, `current_phase`, `artifacts{8 keys}`, `active_skills`), mapped from the final state.
- `artifacts` maps today's 8 keys to the closest Deliverables. `source_discovery_doc` and `source_mermaid_diagram` become the current-state section of `requirements`.
- `total_loop_iterations` = total Revisions + 1.
- `active_skills` = the Skills loaded during the Execution.
The route switches to the new graph in increment 7, behind `AGENT_ENGINE=hierarchical` (default `legacy` until increment 9).
### Recording
`app/execution/recorder.py` reuses `DatabaseManager` tables until candidate 03:
- `workflow_executions`: one row per Execution, updated at each interrupt and at completion.
- `adk_artifacts`: one row per Deliverable submission. Rows for earlier Revisions stay as history.
- `orchestrator_review_logs`: one row per Ledger event. `review_status` holds the event type, `reviewer_agent` holds the role or `user`, and `feedback` holds the reason or feedback.
- `adk_sessions`: the latest Execution summary for the Session (D9).
### Settings
Add to `app/config.py` and `.env.example`:
- `CLOUD_PROVIDER=gcp`
- `AGENT_ENGINE=legacy|hierarchical`
- `ORCHESTRATOR_BUDGET=40`, `SPECIALIST_BUDGET=15`, `MAX_REVISIONS=3`
- `CHECKPOINTER=sqlite|postgres`, `CHECKPOINT_DB_PATH=./checkpoints.db`
- `A2A_HOST=0.0.0.0`, `A2A_PORT=8000`
- `MCP_ENABLE_PREVIEW=true`, `MCP_QUOTA_PROJECT` (defaults to the ADC project)
Model API keys follow LiteLLM conventions (`GEMINI_API_KEY` / Vertex ADC).
### Dependencies
Add to `requirements.txt`:
- `google-adk>=1.25` (Skills are Experimental: pin the exact version in the lock file you use)
- `litellm~=1.74`
- `a2a-sdk>=0.3`
- `aiohttp>=3.10`
- `langgraph-checkpoint-sqlite`
- `langgraph-checkpoint-postgres`
Keep `langgraph` and upgrade it to a version with `interrupt()` and `Send`. The `terraform` CLI is added to the Dockerfile image.
## Testing
**Deterministic**, with `FakeAgentRuntime`, which replays scripted `Outcome`s and tool calls per role and task. There are no model calls and no network.
- **`tests/execution/test_ledger.py`** (pure):
- Ready work respects both thresholds: same-Phase `submitted` and cross-Phase `approved`.
- Each refusal case: `delegate` on unready work, `request_approval` with unsubmitted Deliverables, `finish` before Phase 4 is approved.
- `reopen` cascades to everything that depends on the Deliverable and withdraws approved Phases. The cap produces an escalation.
- Pre-approved `request_approval` grants and records `pre-approved`.
- **`tests/execution/test_graph.py`**, with the fake runtime and `MemorySaver`:
- **Happy path, pre-approved:** every Deliverable submitted, all 4 Phases approved, files written under `deliverables/executions/{id}/` (tmp dir).
- **Parallel start:** after `product_selection` is submitted, one Orchestrator turn can delegate `architecture_diagram` and `design_recommendations` together, and both run.
- **One voice:** questions from two parallel Specialists produce a single interrupt holding both.
- **Interactive pause and resume:** an Approval interrupt stops the graph. Resuming with `approve` continues, and resuming with `changes` plus feedback reopens.
- **Validation failure:** reopening `deployment_guidance` withdraws Phase 2's Approval, re-delegates the IaC author with the findings, and asks for Phase 2 Approval again.
- **Budgets:** an exhausted budget hands control to the user with a summary.
- **Cloud read:** a tool with `allow_cloud_read=None` produces one permission question. `False` makes the tool refuse.
- **Clean Executions:** two Executions in one Session share no state.
- **`tests/agents/test_roles.py`:** every `agents/*.md` parses, and every role maps to a Deliverable kind (or `orchestrator`/`pillar-advisor`), with Skills that exist in the provider bundle.
- **`tests/test_vendored_skills.py`:**
- Vendored Skills match `VENDOR.md`'s commit (by checksum).
- Every Skill under `skills/` passes Agent Skills frontmatter rules: kebab-case `name` matching its directory, `description` ≤ 1024 chars.
- **`tests/agents/test_adk_runtime.py`:** builds an ADK agent for one role with a stub model. It confirms that the `SkillToolset` holds only that role's Skills, that the tool list matches the role's tiers and MCP servers, and that each `McpToolset` has the server's `tool_filter`. It does not call a model.
- **`tests/tools/test_mcp.py`:** uses a local fake MCP server over streamable HTTP, started in the test.
- Tools outside `allow` are never exposed.
- An allowed tool whose `readOnlyHint` isn't true makes the server fail to load.
- A `cloud_read` MCP tool call without permission is blocked by `before_tool_callback`.
- An unreachable server returns a structured discovery gap and doesn't raise.
- `google_adc` headers include `X-Goog-User-Project`.
- **`tests/test_provider_bundle.py`:** every `mcp_servers` entry has `tier` and a non-empty `allow`. `developer_knowledge` is the only `general` server. Every `role_mcp_servers` id exists.
- **`tests/a2a/test_executor.py`:** A2A task → `input-required` on interrupt → `DataPart` approval resumes, using the fake runtime. A free-text "approve" does **not** grant.
- **`tests/test_agent_api.py`:** the `/generate` response keys are unchanged under both `AGENT_ENGINE` values.
**Evaluation** (`eval/`):
- **Harness:** runs `benchmark_cases.json` in pre-approved mode with the real `AdkRuntime`. Each case adds `scripted_answers: {question_pattern: answer}`, and the pre-approved `await_user` uses these answers before falling back to assumptions.
- **Scoring:** existing rubric metrics, plus checks that the Solution guide has every `output-template.md` heading and that the product selection names at least one alternative with pros and cons.
- **Not in the default test run.** It lives under `eval/` and is marked.
## Build increments
These are ordered, and each leaves the suite green. They are called *increments* so they don't clash with the glossary's **Phase**.
1. **Environment and baseline.**
- Create a venv and install dependencies, plus `terraform`.
- Run the current suite and record the baseline under `## Comments`.
- Create the branch.
2. **Vendored Skills and provider bundle.**
- Add `skills/vendor/google/` at `81a31a6` with `VENDOR.md` and the licence, plus the sync script.
- Add `providers/gcp.yaml` and `app/providers.py`.
- Add the vendoring and frontmatter tests.
3. **Deliverables and Ledger,** test-first: `deliverables.py`, `ledger.py`, `test_ledger.py`.
4. **Execution graph,** test-first:
- `runtime.py` (the interface and outcome types), `fake_runtime.py`.
- `state.py`, `tools.py`, `graph.py`, `recorder.py`.
- Checkpointer settings.
- `test_graph.py`.
5. **ADK runtime and role files:** `adk_runtime.py`, `roles.py`, all `agents/*.md`, `test_roles.py`, `test_adk_runtime.py`.
6. **Tools and remote MCP servers:**
- General and cloud-read local tools (`terraform` in a temp dir).
- `app/tools/mcp.py`, `google_adc` auth and the `McpToolset` mapping.
- **Allowlists:** run `tools/list` against each Google Cloud MCP server in a sandbox project, review the read-only tools, and write the `allow` lists into `providers/gcp.yaml`, recording the reviewed tool lists and date under `## Comments`.
- `docs/operations/mcp-access.md`: the APIs to enable, viewer roles, and blocking read-write MCP use with IAM deny.
- The `skills/cloud-discovery` and `skills/iac-validation` Skills.
- The tests in `test_mcp.py` and `test_provider_bundle.py`.
7. **Interfaces:**
- The A2A server and executor, `agent_card.json`, `test_executor.py`.
- `/generate` behind `AGENT_ENGINE`, and the Dockerfile exposing :8000 and :8080.
8. **Evaluation:** scripted answers, template-heading scoring, and a real-model eval run with results recorded under `## Comments`. Proceed to 9 only when the pass rate meets the current harness threshold (≥ 80% per case).
9. **Cleanup** (carried over from the superseded spec):
- **Delete:** `app/tools/gcp_scanner.py`, `app/tools/mcp_developer_knowledge.py` (replaced by remote MCP servers), `app/adk/`, `app/nodes/`, `app/states/`, `app/workflows/gcp_architecture_graph.py`, `app/skills/` (the loader and the 5 non-compliant Skills), `tests/test_graph_workflow.py`, the ADK and orchestrator tests in `tests/test_adk_architecture.py` (keeping the DB test as `tests/test_database.py`), and `test_adk_mcp_function_tools`.
- **Fix:** remove the unreachable duplicate `return` in `routes.py`. Rename `ADK_MAX_LOOP_ITERATIONS` → removed, and delete `ADK_ENABLE_ARTIFACT_STORE`.
- **Update:**
- Make `AGENT_ENGINE=hierarchical` the only engine.
- Derive `app/card.py` / `agent_card.json` capabilities from `DEPENDENCY_GRAPH` and the role files.
- Rewrite `workflow.yaml` phases to upstream's 4 with their Deliverables.
- Rewrite the README around the Orchestrator, Specialists, Skills and A2A.
- **Stop** the writes to shared paths (`deliverables/{as-is,target,validation,guides}/`, `docs/*.md`, `terraform/main.tf`).
- Push the branch.
## Acceptance
- [ ] **Upstream parity:** a pre-approved evaluation run produces a Solution guide containing every heading of the upstream `output-template.md`, and all benchmark cases pass the threshold.
- [ ] **Single source for the workflow:** Phases, Deliverables and Dependencies are listed only in `app/execution/deliverables.py`, and card and manifest data is derived from it.
- [ ] **No framework types outside `app/agents/adk_runtime.py`:** `rg -n "google\.adk" app | grep -v adk_runtime.py` is empty. After increment 9, `rg -n "app\.adk|OrchestratorLoopAgent|compat" app eval tests` is empty.
- [ ] **Vendored Skills unchanged:** they match the `81a31a6` checksums, and every Skill under `skills/` passes the frontmatter rules.
- [ ] **Approvals only by structured decision:** no path grants an Approval from free text (covered by tests).
- [ ] **Nothing changes cloud resources:** no tool can run `terraform apply`, a deploy or a script (`run_skill_script` absent from the tool lists). Cloud-read tools, MCP included, are unreachable without permission.
- [ ] **Discovery uses the provider's remote MCP servers:** current state comes from the Cloud provider's hosted MCP servers. No module under `app/` calls a cloud provider's REST API directly (`rg -n "googleapis.com/(compute|storage|run|sqladmin|pubsub)/v" app` is empty). Every exposed MCP tool is on a reviewed read-only allowlist.
- [ ] **Deterministic suite:** every test above passes with no network access.
- [ ] **`/generate` contract:** response keys are unchanged.
## Risks
- **Specialists submitting without asking (D32).** A Specialist might submit without getting the user's conversational OK. Mitigation: role files require confirmation through `needs_input` before `submit_deliverable`, and the evaluation checks that submissions follow a question. If it's still observed, fall back to Approval per Deliverable (the Q31 alternative).
- **ADK Skills are Experimental.** The `SkillToolset` API may change. It's confined to `adk_runtime.py`, and Deep Agents is the planned second implementation.
- **Parallel Specialists and rate limits.** Fan-out multiplies concurrent model calls. Cap concurrency per Execution (default 4) in `graph.py`.
- **Upstream Skill drift.** Upstream edits may assume a different workflow. Re-sync only through the script, and review the diff against `agents/*.md` and `deliverables.py`.
- **Two frameworks in one process.** LangGraph and ADK both manage async state. Keep ADK sessions in memory per run, and never put ADK objects in graph state.
- **Remote MCP server availability and drift.** Several servers are Preview (Asset Inventory, Network Intelligence, Billing), each needs its product API enabled in the user's project, and tool sets can change. Mitigations:
- allowlists are reviewed, not discovered at run time
- a changed `readOnlyHint` fails the server loudly
- preview servers can be switched off
- an unavailable server becomes a recorded discovery gap, not a failure
- **Uneven hosting across Cloud providers.** AWS hosts a managed MCP server. Azure's general MCP server is self-hosted. The bundle's `url` covers both, but "hosted by the Cloud provider" isn't uniform.
## Comments