Compare commits
1 Commits
main
...
feat/solut
| Author | SHA1 | Date | |
|---|---|---|---|
| 4fefe0ab33 |
361
.scratch/phase-pipeline/spec.md
Normal file
361
.scratch/phase-pipeline/spec.md
Normal 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
470
.scratch/solution-architect-agent/spec.md
Normal 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
|
||||
19
CLAUDE.md
Normal file
19
CLAUDE.md
Normal file
@@ -0,0 +1,19 @@
|
||||
# CLAUDE.md
|
||||
|
||||
## Direction
|
||||
|
||||
Before designing or restructuring agent code, read `docs/adr/`. ADR-0002 is the anchor: behaviour lives in Agent Skills (agentskills.io format) loaded by the framework's own skill runtime, and this repo builds the tooling around them. Google Cloud is the first Cloud provider, not the only one, so cloud-specific knowledge belongs in Skills and tooling, never in the agent's core. Reach cloud environments and cloud documentation through the provider's hosted remote MCP servers, not hand-written API clients. The current build plan is `.scratch/solution-architect-agent/spec.md`.
|
||||
|
||||
## Agent skills
|
||||
|
||||
### Issue tracker
|
||||
|
||||
Issues and specs live as local markdown files under `.scratch/<feature-slug>/`. See `docs/agents/issue-tracker.md`.
|
||||
|
||||
### Triage labels
|
||||
|
||||
Default five-role vocabulary (`needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, `wontfix`), recorded as a `Status:` line in each issue file. See `docs/agents/triage-labels.md`.
|
||||
|
||||
### Domain docs
|
||||
|
||||
Single-context: one `CONTEXT.md` and `docs/adr/` at the repo root. See `docs/agents/domain.md`.
|
||||
65
CONTEXT.md
Normal file
65
CONTEXT.md
Normal file
@@ -0,0 +1,65 @@
|
||||
# Cloud Solution Architecture
|
||||
|
||||
Turns a request for a cloud workload into a reviewed solution architecture: requirements, a target design with diagram and infrastructure-as-code, validation results, and a packaged guide. Google Cloud is the first Cloud provider.
|
||||
|
||||
## Language
|
||||
|
||||
### Running the workflow
|
||||
|
||||
**Workflow request**:
|
||||
The free-text description of the workload a caller wants architected; the opening input to an Execution.
|
||||
_Avoid_: prompt, query, request summary
|
||||
|
||||
**Execution**:
|
||||
One solution-architecture engagement for a single Workflow request. It lasts from the first request to an approved Solution guide, and may span many user turns.
|
||||
_Avoid_: run, job, workflow execution
|
||||
|
||||
**Session**:
|
||||
A caller-supplied grouping of Executions. A Session never feeds earlier outputs into a later Execution; each Execution starts clean.
|
||||
_Avoid_: conversation, context
|
||||
|
||||
**Phase**:
|
||||
A named grouping of Deliverables: requirements discovery, solution architecture, solution validation, or packaging. A Phase is complete when all its Deliverables are submitted and the user has granted the Phase's Approval.
|
||||
_Avoid_: step, node, stage, pipeline
|
||||
|
||||
**Approval**:
|
||||
A user's explicit acceptance of a Phase's Deliverables, required before work that depends on them may start. An Execution is interactive by default. A **pre-approved** Execution treats its Approvals as already granted.
|
||||
_Avoid_: sign-off, confirmation, gate (in prose)
|
||||
|
||||
**Revision**:
|
||||
Reopening a Deliverable because validation found a problem or the user asked for a change. It returns that Deliverable and everything that depends on it to draft, and withdraws any Approval that covered them. Revisions per Deliverable are limited, and past the limit the user decides.
|
||||
_Avoid_: review round, retry, loop iteration
|
||||
|
||||
### Agents
|
||||
|
||||
**Orchestrator**:
|
||||
The top-level agent of an Execution, and the only agent that talks to the user. It reasons about which Deliverables are ready to produce, delegates them to Specialists, and asks the user for Approvals.
|
||||
_Avoid_: supervisor, manager, root agent, router
|
||||
|
||||
**Specialist**:
|
||||
An agent responsible for producing one kind of Deliverable, such as the product selector or the diagrammer. A Specialist may delegate independent parts of its work to further Specialists.
|
||||
_Avoid_: sub-agent, worker, phase agent, node
|
||||
|
||||
### Portability
|
||||
|
||||
**Skill**:
|
||||
A self-contained package of domain guidance: instructions plus optional references, assets and scripts, in the Agent Skills format. It is the portable unit of the agent's behaviour and is loaded on demand.
|
||||
_Avoid_: prompt template, instructions, playbook
|
||||
|
||||
**Cloud provider**:
|
||||
The hyperscaler a solution architecture targets, such as Google Cloud. Each Cloud provider brings its own Skills, plus the MCP servers it hosts for discovering a user's current environment and grounding recommendations in its documentation.
|
||||
_Avoid_: hyperscaler, cloud, platform, target
|
||||
|
||||
### Outputs
|
||||
|
||||
**Deliverable**:
|
||||
A document an Execution produces: requirements, technical decomposition, product selection, architecture diagram, architecture description, design recommendations, deployment guidance, validation results, or the Solution guide. Each Deliverable depends on others being approved first.
|
||||
_Avoid_: artifact, output file
|
||||
|
||||
**Dependency**:
|
||||
The relationship between two Deliverables in which one can only be produced once the other has been accepted, for example an architecture diagram depending on the product selection. Within a Phase, a submitted Deliverable counts as accepted. Across Phases, only the earlier Phase's Approval does.
|
||||
_Avoid_: prerequisite, edge, upstream
|
||||
|
||||
**Solution guide**:
|
||||
The packaged Deliverable that assembles every other Deliverable into the Cloud provider's solution architecture template.
|
||||
_Avoid_: final report, package
|
||||
@@ -2,6 +2,12 @@
|
||||
|
||||
A production-ready Google Cloud Solution Architecture Agent built using **Google ADK (Agent Development Kit)**, **LangChain**, and **LangGraph**, featuring a multi-agent orchestration graph with iterative review loops, dynamic local skill loading, PostgreSQL database persistence, a Starlette REST API server, and an offline evaluation harness.
|
||||
|
||||
> **Direction and current state (2026-09-15).** This agent is being built to replace the [`google-cloud-solution-architecture`](https://github.com/google/skills/blob/main/skills/cloud/google-cloud-solution-architecture/SKILL.md) skill with something that runs on any agent framework and, later, targets any Cloud provider. Google Cloud is the first provider.
|
||||
> - **Target:** behaviour lives in [Agent Skills](https://agentskills.io/specification), loaded by a framework's own skill runtime (Google ADK `SkillToolset`, or LangChain Deep Agents skills). This repo supplies the tooling those Skills call. See [ADR-0002](docs/adr/0002-skills-are-the-portable-unit-of-behaviour.md).
|
||||
> - **Today:** the `google.adk` classes below are local compatibility stubs. The real `google-adk` package isn't used, and Phase outputs are fixed templates, not model-generated.
|
||||
>
|
||||
> Vocabulary is defined in [CONTEXT.md](CONTEXT.md); decisions are recorded in [docs/adr/](docs/adr/).
|
||||
|
||||
---
|
||||
|
||||
## 🌟 Architecture & Features
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
---
|
||||
status: superseded by ADR-0003
|
||||
---
|
||||
|
||||
# LangGraph orchestrates Phases, not Google ADK
|
||||
|
||||
Phases are orchestrated by a LangGraph `StateGraph`, as the KAB Workflow Agent standard (`kyndryl-agent-builder/docs/KAB_WORKFLOW_AGENT_IMPLEMENTATION_GUIDE.md`) requires, rather than by Google ADK `Runner`/`LoopAgent`. The earlier "ADK" orchestration was a local compatibility layer that never used the real `google-adk` package, so it added a pretend seam with one adapter and diverged from the platform standard; we deleted it. Real ADK remains welcome *inside* a Phase (e.g. an `LlmAgent` for LLM-driven design) — just not as the thing that sequences Phases.
|
||||
|
||||
> **Reopened (2026-09-15).** [ADR-0002](0002-skills-are-the-portable-unit-of-behaviour.md) makes Skills, loaded by a framework's own skill runtime, the portable unit of behaviour. This ADR assumed our code sequences the Phases. If the Skill sequences them instead, the choice here reduces to which skill runtime to wire first, and "ADK only inside a Phase" no longer holds. Still true regardless: the compat stubs are fiction and should go.
|
||||
|
||||
## Considered Options
|
||||
|
||||
- **Real `google-adk` as orchestrator**: rejected; departs from the KAB workflow standard and would require async event-stream plumbing for no capability LangGraph lacks (conditional edges cover the Review round).
|
||||
- **Plain in-process pipeline, no framework**: rejected; neither KAB template uses it, and LangGraph already gives us conditional routing and a path to checkpointing if a human approval gate is added later.
|
||||
25
docs/adr/0002-skills-are-the-portable-unit-of-behaviour.md
Normal file
25
docs/adr/0002-skills-are-the-portable-unit-of-behaviour.md
Normal file
@@ -0,0 +1,25 @@
|
||||
---
|
||||
status: accepted
|
||||
---
|
||||
|
||||
# Skills are the portable unit of behaviour
|
||||
|
||||
This agent exists to replace the [`google-cloud-solution-architecture`](https://github.com/google/skills/blob/main/skills/cloud/google-cloud-solution-architecture/SKILL.md) skill with an agent that runs anywhere. Its solution-architecture behaviour lives in Skills that follow the [Agent Skills specification](https://agentskills.io/specification) (`SKILL.md` plus `references/`, `assets/` and `scripts/`). The agent framework's own skill runtime loads them: Google ADK's `SkillToolset`, or LangChain's skills pattern (Deep Agents `skills=` / `SkillsMiddleware`). Our code supplies the tooling those Skills call on: grounding, IaC validation and environment scanning. It doesn't restate the work the Skills describe.
|
||||
|
||||
Google Cloud is the first and, for now, only Cloud provider. Other hyperscalers should be addable as new Skills plus new tooling, without rewriting the agent's core.
|
||||
|
||||
## Refinements (2026-09-15)
|
||||
|
||||
- **Hybrid sequencing.** Our code owns Phase boundaries and the Approvals between Phases. Within a Phase, the Skill, run by the skill runtime, decides the work. We rejected letting the Skill sequence everything: Phase checkpoints give us resumable, recordable, testable progress. We rejected code-owned Phase bodies because they would bypass the Skill.
|
||||
- **Hierarchical and ReAct, not linear.** Work isn't a fixed chain. Agents are organised in a hierarchy that follows the dependencies between Deliverables. Work whose dependencies are satisfied may proceed independently, and every agent reasons and acts in a ReAct loop of thought, tool call and observation. An Orchestrator sits above one Specialist per Deliverable, and a Specialist fans out further only where its work splits into independent parts. Code declares the dependency graph, and the Orchestrator chooses what to do next within it. The engine is recorded in [ADR-0003](0003-langgraph-holds-state-adk-agents-reason.md).
|
||||
- **Interactive by default.** Executions are multi-turn with user Approval, as the upstream Skill requires. A pre-approved mode serves batch and evaluation runs.
|
||||
- **Vendored upstream Skill.** `google-cloud-solution-architecture` is copied unmodified and pinned to an upstream commit. Our additions live in separate Skills for our own tooling, so upstream changes arrive as a diff.
|
||||
- **Discovery through the provider's own MCP servers.** Current-state discovery and documentation grounding go through the remote MCP servers each Cloud provider hosts, for example Google Cloud's Resource Manager, Asset Inventory, Compute, GKE and Developer Knowledge servers, and the AWS MCP Server. Code carries no hand-written cloud API clients. Only reviewed read-only MCP tools are exposed, and reading a user's cloud requires their permission.
|
||||
- **Cloud provider as configuration.** A Cloud provider is a configured bundle of Skills, tools and remote MCP servers. There's no provider interface in code until a second Cloud provider exists, but core module names carry no provider prefix.
|
||||
|
||||
## Consequences
|
||||
|
||||
- Framework-specific code is confined to wiring a skill runtime and registering tools, so moving to another framework is a new wiring, not a rewrite.
|
||||
- Skills in `app/skills/` must conform to the specification: kebab-case `name` matching the directory, a `description` of when to use the Skill, and no custom `phase:` key. They don't today.
|
||||
- This reopens [ADR-0001](0001-langgraph-orchestrates-phases-not-google-adk.md) and the Phase pipeline spec (`.scratch/phase-pipeline/spec.md`). [ADR-0003](0003-langgraph-holds-state-adk-agents-reason.md) settles the engine and supersedes ADR-0001.
|
||||
- Both known skill runtimes are young: ADK marks Skills *Experimental* (Python ≥ 1.25.0), and Deep Agents skills are new. Pin versions.
|
||||
25
docs/adr/0003-langgraph-holds-state-adk-agents-reason.md
Normal file
25
docs/adr/0003-langgraph-holds-state-adk-agents-reason.md
Normal file
@@ -0,0 +1,25 @@
|
||||
---
|
||||
status: accepted
|
||||
supersedes: ADR-0001
|
||||
---
|
||||
|
||||
# LangGraph holds Execution state; ADK agents reason and act
|
||||
|
||||
An Execution is a hierarchy of ReAct agents shaped by the dependencies between Deliverables ([ADR-0002](0002-skills-are-the-portable-unit-of-behaviour.md)).
|
||||
|
||||
- **LangGraph holds the Execution.** A LangGraph `StateGraph` holds the dependency graph and each Deliverable's state. It checkpoints after every step and pauses with `interrupt()` whenever the user must answer or approve. This follows the KAB workflow standard and the platform's `langgraph-workflow-agent`.
|
||||
- **ADK agents reason and act.** The Orchestrator and every Specialist is an ADK `LlmAgent`, each running its own ReAct loop, with a `SkillToolset` for progressive Skill loading. This follows the platform's `google-adk-base-agent/app/skills_integration.py`.
|
||||
- **Agents sit behind one interface.** Each agent runs behind a single agent-runtime interface, so LangChain Deep Agents can be added as a second implementation without touching the graph.
|
||||
|
||||
The dependency graph is declared as data in code. The Orchestrator picks what to do next through tools that refuse work whose dependencies aren't met. Dependencies are guaranteed by code, while ordering and parallelism are the model's choice.
|
||||
|
||||
## Considered Options
|
||||
|
||||
- **ADK end to end** (root `LlmAgent` with `sub_agents`/`AgentTool`, and ADK sessions for pauses): rejected. It departs from the KAB workflow standard, and LangGraph's checkpointed `interrupt()` is the proven platform path for resumable user Approvals.
|
||||
- **Deep Agents end to end** (`create_deep_agent(skills=, subagents=)`): viable, and it compiles to LangGraph. It was deferred rather than rejected: ADK `SkillToolset` already has working prior art on this platform, and Deep Agents stays reachable as the second implementation of the agent-runtime interface.
|
||||
- **Code runs the dependency graph with no Orchestrator model**: rejected. The top level wouldn't be ReAct, and the Skill's conversational flexibility (such as skipping work the user says is already approved) would need hand-coding.
|
||||
|
||||
## Consequences
|
||||
|
||||
- There are two frameworks in one process. Framework types stay inside the agent-runtime implementation and never appear in graph state.
|
||||
- ADK Skills are *Experimental* (Python ≥ 1.25.0), so pin the version.
|
||||
51
docs/agents/domain.md
Normal file
51
docs/agents/domain.md
Normal file
@@ -0,0 +1,51 @@
|
||||
# Domain Docs
|
||||
|
||||
How the engineering skills should consume this repo's domain documentation when exploring the codebase.
|
||||
|
||||
## Before exploring, read these
|
||||
|
||||
- **`CONTEXT.md`** at the repo root, or
|
||||
- **`CONTEXT-MAP.md`** at the repo root if it exists: it points at one `CONTEXT.md` per context. Read each one relevant to the topic.
|
||||
- **`docs/adr/`**: read ADRs that touch the area you're about to work in. In multi-context repos, also check `src/<context>/docs/adr/` for context-scoped decisions.
|
||||
|
||||
If any of these files don't exist, **proceed silently**. Don't flag their absence; don't suggest creating them upfront. The `/domain-modeling` skill (reached via `/grill-with-docs` and `/improve-codebase-architecture`) creates them lazily when terms or decisions actually get resolved.
|
||||
|
||||
## File structure
|
||||
|
||||
Single-context repo (most repos):
|
||||
|
||||
```
|
||||
/
|
||||
├── CONTEXT.md
|
||||
├── docs/adr/
|
||||
│ ├── 0001-event-sourced-orders.md
|
||||
│ └── 0002-postgres-for-write-model.md
|
||||
└── src/
|
||||
```
|
||||
|
||||
Multi-context repo (presence of `CONTEXT-MAP.md` at the root):
|
||||
|
||||
```
|
||||
/
|
||||
├── CONTEXT-MAP.md
|
||||
├── docs/adr/ ← system-wide decisions
|
||||
└── src/
|
||||
├── ordering/
|
||||
│ ├── CONTEXT.md
|
||||
│ └── docs/adr/ ← context-specific decisions
|
||||
└── billing/
|
||||
├── CONTEXT.md
|
||||
└── docs/adr/
|
||||
```
|
||||
|
||||
## Use the glossary's vocabulary
|
||||
|
||||
When your output names a domain concept (in an issue title, a refactor proposal, a hypothesis, a test name), use the term as defined in `CONTEXT.md`. Don't drift to synonyms the glossary explicitly avoids.
|
||||
|
||||
If the concept you need isn't in the glossary yet, that's a signal: either you're inventing language the project doesn't use (reconsider) or there's a real gap (note it for `/domain-modeling`).
|
||||
|
||||
## Flag ADR conflicts
|
||||
|
||||
If your output contradicts an existing ADR, surface it explicitly rather than silently overriding:
|
||||
|
||||
> _Contradicts ADR-0007 (event-sourced orders), but worth reopening because…_
|
||||
30
docs/agents/issue-tracker.md
Normal file
30
docs/agents/issue-tracker.md
Normal file
@@ -0,0 +1,30 @@
|
||||
# Issue tracker: Local Markdown
|
||||
|
||||
Issues and specs for this repo live as markdown files in `.scratch/`.
|
||||
|
||||
## Conventions
|
||||
|
||||
- One feature per directory: `.scratch/<feature-slug>/`
|
||||
- The spec is `.scratch/<feature-slug>/spec.md`
|
||||
- Implementation issues are one file per ticket at `.scratch/<feature-slug>/issues/<NN>-<slug>.md`, numbered from `01`, never a single combined tickets file
|
||||
- Triage state is recorded as a `Status:` line near the top of each issue file (see `triage-labels.md` for the role strings)
|
||||
- Comments and conversation history append to the bottom of the file under a `## Comments` heading
|
||||
|
||||
## When a skill says "publish to the issue tracker"
|
||||
|
||||
Create a new file under `.scratch/<feature-slug>/` (creating the directory if needed).
|
||||
|
||||
## When a skill says "fetch the relevant ticket"
|
||||
|
||||
Read the file at the referenced path. The user will normally pass the path or the issue number directly.
|
||||
|
||||
## Wayfinding operations
|
||||
|
||||
Used by `/wayfinder`. The **map** is a file with one **child** file per ticket.
|
||||
|
||||
- **Map**: `.scratch/<effort>/map.md` (the Notes / Decisions-so-far / Fog body).
|
||||
- **Child ticket**: `.scratch/<effort>/issues/NN-<slug>.md`, numbered from `01`, with the question in the body. A `Type:` line records the ticket type (`research`/`prototype`/`grilling`/`task`); a `Status:` line records `claimed`/`resolved`.
|
||||
- **Blocking**: a `Blocked by: NN, NN` line near the top. A ticket is unblocked when every file it lists is `resolved`.
|
||||
- **Frontier**: scan `.scratch/<effort>/issues/` for files that are open, unblocked, and unclaimed; first by number wins.
|
||||
- **Claim**: set `Status: claimed` and save before any work.
|
||||
- **Resolve**: append the answer under an `## Answer` heading, set `Status: resolved`, then append a context pointer (gist + link) to the map's Decisions-so-far in `map.md`.
|
||||
15
docs/agents/triage-labels.md
Normal file
15
docs/agents/triage-labels.md
Normal file
@@ -0,0 +1,15 @@
|
||||
# Triage Labels
|
||||
|
||||
The skills speak in terms of five canonical triage roles. This file maps those roles to the actual label strings used in this repo's issue tracker.
|
||||
|
||||
| Label in mattpocock/skills | Label in our tracker | Meaning |
|
||||
| -------------------------- | -------------------- | ---------------------------------------- |
|
||||
| `needs-triage` | `needs-triage` | Maintainer needs to evaluate this issue |
|
||||
| `needs-info` | `needs-info` | Waiting on reporter for more information |
|
||||
| `ready-for-agent` | `ready-for-agent` | Fully specified, ready for an AFK agent |
|
||||
| `ready-for-human` | `ready-for-human` | Requires human implementation |
|
||||
| `wontfix` | `wontfix` | Will not be actioned |
|
||||
|
||||
When a skill mentions a role (e.g. "apply the AFK-ready triage label"), use the corresponding label string from this table.
|
||||
|
||||
Edit the right-hand column to match whatever vocabulary you actually use.
|
||||
Reference in New Issue
Block a user