From af1ccd63e3cbcebe547cb4c8193a489ec9a86aff Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Fri, 5 Jun 2026 14:17:32 +0200 Subject: [PATCH] Using mermaid diagram to explain the federation review process Signed-off-by: Daniele Martinoli --- FEDERATION_REQUEST_GUIDE.md | 2 +- FEDERATION_REVIEW_GUIDE.md | 243 ++++++++++++------------------------ 2 files changed, 83 insertions(+), 162 deletions(-) diff --git a/FEDERATION_REQUEST_GUIDE.md b/FEDERATION_REQUEST_GUIDE.md index 071672d..dd6cfd9 100644 --- a/FEDERATION_REQUEST_GUIDE.md +++ b/FEDERATION_REQUEST_GUIDE.md @@ -94,7 +94,7 @@ git commit -m "feat: federate module from " gh pr create --title "feat: federate module" --label "federation" ``` -The **`federation`** label triggers CI. Full check list: [FEDERATION_REVIEW_GUIDE.md](FEDERATION_REVIEW_GUIDE.md#automated-validation-recommended). +The **`federation`** label triggers CI. Full check list: [FEDERATION_REVIEW_GUIDE.md#checks](FEDERATION_REVIEW_GUIDE.md#checks). --- diff --git a/FEDERATION_REVIEW_GUIDE.md b/FEDERATION_REVIEW_GUIDE.md index e98f839..e127c09 100644 --- a/FEDERATION_REVIEW_GUIDE.md +++ b/FEDERATION_REVIEW_GUIDE.md @@ -1,205 +1,126 @@ # Federation Review Guide -Step-by-step guide for maintainers evaluating a federation PR. - -Federation means referencing an **external agentic pack** in our catalog. The code stays in the external repo — we don't copy or modify it. Users install federated packs directly from the external repo via Lola. +Evaluate federation PRs that list an **external Lola pack** in our catalog. Code stays in the contributor's repo; users install via [Lola](https://github.com/LobsterTrap/lola). Contributors: [FEDERATION_REQUEST_GUIDE.md](FEDERATION_REQUEST_GUIDE.md). + +> **Lola `ref`:** Required here as a 40-character commit SHA for CI/catalog pinning. Lola ignores it at install until [lola#180](https://github.com/LobsterTrap/lola/issues/180). + +## Review process + +```mermaid +flowchart TB + subgraph input ["Input"] + PR["Federation PR
(marketplace + plugins.json + .catalog/)"] + end + + subgraph automated ["Automated (federation label)"] + CI["validate_federation.py"] + CAT["validate_federation_catalog.py"] + PR --> CI --> CAT + end + + subgraph maintainer ["Maintainer"] + PATH{Path} + SKILL["/federation-review"] + GUIDE["This guide"] + M1["License + owner/contact"] + M2["AI agent compatibility
(CLAUDE.md, Cursor, ChatGPT)"] + M3["Catalog quality spot-check"] + CAT --> PATH + PATH -->|recommended| SKILL --> M1 + PATH -->|manual| GUIDE --> M1 + M1 --> M2 --> M3 + end + + subgraph decision ["Decision"] + D{Outcome} + M3 --> D + D -->|pass| OK["Approve & merge"] + D -->|fixable| CR["Comment · federation/changes-requested"] + D -->|block| RJ["Reject · federation/rejected"] + end + + OK --> VERIFY["Optional: lola market ls"] +``` -Contributors: see [FEDERATION_REQUEST_GUIDE.md](FEDERATION_REQUEST_GUIDE.md) for how to open a federation PR. +End-to-end contributor flow: [FEDERATION_REQUEST_GUIDE.md#process](FEDERATION_REQUEST_GUIDE.md#process). -> **Lola `ref` field:** Marketplace `ref` is required here for validation and catalog pinning, but **Lola ignores it at install time** until [LobsterTrap/lola#180](https://github.com/LobsterTrap/lola/issues/180) is fixed. +--- -## Two ways to review +## Checks -| Method | When to use | -|--------|------------| -| **`/federation-review` skill** | Interactive review from Claude Code. Reads the issue, runs validation, guides you through manual checks, and creates the registration PR if approved. | -| **Manual (this guide)** | Step-by-step commands when you prefer to review without Claude Code. | +| Check | Blocks merge? | Automated | Tool | +|-------|---------------|-----------|------| +| Public access + valid `ref` SHA | Yes | Yes | `validate_federation.py` | +| Lola module schema | Yes | Yes | `validate_federation.py` | +| Tier 1 (agentskills.io) | Yes | Yes | `validate_federation.py` | +| Tier 2 (design principles) | **No** — warns | Yes | `validate_federation.py` | +| MCP pinning + credentials | Yes when applicable | Yes | `validate_federation.py` | +| Credential scan (gitleaks) | Yes | Yes | `validate_federation.py` | +| Catalog cross-check | Yes | Yes | `validate_federation_catalog.py` | +| License (Apache-2.0–compatible) | Yes | No | Manual | +| AI agent compatibility | Yes | No | Manual | +| LLM security scan | On-demand | No | Maintainer-triggered | -CI also runs automated validation on any PR with the `federation` label (see [CI automation](#ci-automation) below). +CI workflow: `.github/workflows/federation-validation.yml` — runs on PRs with the **`federation`** label only; posts a summary comment. --- -## Automated validation (recommended) - -Steps 1–6 can be run with a single command from the agentic-collections repo root: +## Run validation ```bash -# Full pack at repo root, pinned to a commit SHA -uv run python scripts/validate_federation.py --ref <40-character-commit-sha> - -# Pack in a subdirectory -uv run python scripts/validate_federation.py --ref --pack-path - -# JSON output (for CI) -uv run python scripts/validate_federation.py --ref --json -``` - -The script checks: commit SHA format, clone access at the pinned commit, Lola module schema, Tier 1, Tier 2, MCP version pinning, and gitleaks. +# Pack validation (same as CI) +uv run python scripts/validate_federation.py --ref <40-char-sha> +uv run python scripts/validate_federation.py --ref --pack-path --json -### Catalog cross-check (in-repo federation artifacts) - -When the PR includes `federation/modules//.catalog/`, verify the catalog matches the external pack at `ref`: - -```bash +# Catalog + marketplace alignment (when PR includes federation/modules//.catalog/) uv run python scripts/validate_federation_catalog.py \ --module-name \ --repo-url \ - --ref \ + --ref \ --pack-path \ --module-json '' ``` -This checks collection compliance, **skill roster parity** with the linked repo at `ref`, `docs/plugins.json` title alignment, and marketplace version/description/repository consistency. - -If all pass, the pack is ready for manual review (steps 7–8). +**Manual spot-checks** (not fully automated): ---- - -## Manual steps +- [ ] Owner/contact provided on the PR or issue +- [ ] LICENSE in external repo is Apache-2.0–compatible +- [ ] Declared agents supported (`CLAUDE.md` routing, Cursor/ChatGPT config as claimed) +- [ ] Destructive skill operations require human confirmation +- [ ] Full pack at `path` is appropriate — no per-skill subset in marketplace YAML -### Step 1: Verify access and basic info - -- [ ] Repository URL is reachable and public -- [ ] `ref` is a 40-character commit SHA (not a branch or tag) and exists: `git ls-remote ` -- [ ] Owner/contact information is provided -- [ ] License file exists in the repo and is compatible with Apache 2.0 (e.g., Apache-2.0, MIT, BSD-2-Clause, BSD-3-Clause) +Clone at pinned commit when inspecting by hand: ```bash git clone --no-checkout /tmp/federation-review -cd /tmp/federation-review -git checkout -``` - -### Step 2: Verify Lola module schema - -The module entry in `marketplace/rh-agentic-collection.yml` must have the required Lola fields: - -| Field | Required | Description | -|-------|----------|-------------| -| `name` | Yes | Module identifier | -| `description` | Yes | Brief description | -| `version` | Yes | Module version | -| `repository` | Yes | Git URL to the external repo | -| `ref` | Yes (project extension) | 40-character commit SHA (required by this repo; **Lola ignores until [lola#180](https://github.com/LobsterTrap/lola/issues/180)**) | - -All skills under the pack's `path` are validated and installed — the same scope as in-repo marketplace modules. - -### Step 3: Validate skills — Tier 1 (agentskills.io spec) - -Tier 1 is **mandatory**. Run the linter on each skill: - -```bash -# From the agentic-collections repo root: -for skill_dir in /tmp/federation-review/skills/*/; do - ./scripts/run-skill-linter.sh "$skill_dir" -done -``` - -**Result:** All skills must pass (exit code 0). Warnings are acceptable; errors block federation. - -### Step 4: Validate skills — Tier 2 (design principles) - -Tier 2 is **mandatory**. Run the design principles validator: - -```bash -uv run python scripts/validate_skill_design.py /tmp/federation-review -``` - -See [SKILL_DESIGN_PRINCIPLES.md](../SKILL_DESIGN_PRINCIPLES.md) for the full list of design principles. - -### Step 5: Validate MCP configuration - -If the pack uses MCP servers (`mcps.json` is not empty): - -- [ ] **No `:latest` tags** — All container images must use pinned versions or SHAs -- [ ] **No hardcoded credentials** — All secrets use `${ENV_VAR}` format - -```bash -grep -r ":latest" mcps.json && echo "FAIL: found :latest" || echo "PASS: no :latest" -``` - -### Step 6: Security review - -- [ ] No credentials or secrets in any file -- [ ] Destructive operations have human-in-the-loop confirmation - -```bash -gitleaks detect --source /tmp/federation-review --no-git --no-banner --verbose +cd /tmp/federation-review && git checkout +# ... inspect skills/, mcps.json, CLAUDE.md ... +rm -rf /tmp/federation-review ``` -LLM-based security scan is triggered **on-demand** by a maintainer due to cost. - -### Step 7: AI agent compatibility - -Verify the declared AI agent compatibility from the issue: - -- [ ] If **Claude Code** is declared: CLAUDE.md exists with proper intent routing -- [ ] If **Cursor** is declared: check for Cursor-compatible configuration -- [ ] If **ChatGPT** is declared: check for ChatGPT-compatible configuration +--- -### Step 8: Decision +## Decision | Result | Action | |--------|--------| -| All checks pass | Approve — create registration PR with label `federation` | -| Minor issues | Comment on issue with specific fixes, label `federation/changes-requested` | -| Major issues | Reject with explanation, label `federation/rejected`, close issue | - -When approving, add the pack as a new entry in `modules` in `marketplace/rh-agentic-collection.yml` (use the external repository URL; license compatibility is verified from the repo's LICENSE file during review) and link back to the issue. +| All required checks pass | Approve and merge the PR | +| Minor issues | Request specific fixes; label `federation/changes-requested` | +| Major issues | Reject with explanation; label `federation/rejected` | --- -## CI automation +## After merge -PRs with the `federation` label automatically trigger the **Federation Validation** workflow (`.github/workflows/federation-validation.yml`). It: - -1. Identifies federated modules (external repository) in `marketplace/rh-agentic-collection.yml` -2. Runs `scripts/validate_federation.py` on each entry -3. Posts a summary comment on the PR with pass/fail results - -The label-based trigger ensures validation only runs on federation PRs, not on every marketplace YAML change. - ---- - -## Verifying federation with Lola - -After merging a federation PR, verify the module is visible in the marketplace: +Verify listing (replace `main` with PR branch to test pre-merge): ```bash -# Use a unique ephemeral market name for each verification run MARKET="federation-review-$(openssl rand -hex 4)" - -# Add the marketplace (use the raw YAML URL for a specific branch or main) lola market add "$MARKET" https://raw.githubusercontent.com/RHEcosystemAppEng/agentic-collections/main/marketplace/rh-agentic-collection.yml - -# List modules — the federated pack should appear alongside internal packs lola market ls "$MARKET" - -# Clean up when done lola market rm "$MARKET" ``` -To test a PR branch before merging, replace `main` with the branch name in the URL. - ---- - -## Cleanup - -```bash -rm -rf /tmp/federation-review -``` +## Resources -## Quick reference - -| Check | Required | Automated | Tool/Command | -|-------|----------|-----------|--------------| -| Public access | Yes | Yes | `git ls-remote` | -| License compatibility | Yes | No | Manual review | -| Lola module schema | Yes | Yes | `scripts/validate_federation.py` | -| Federation catalog cross-check | Yes | Yes | `scripts/validate_federation_catalog.py` | -| Tier 1 (agentskills.io) | Yes | Yes | `scripts/validate_federation.py` | -| Tier 2 (design principles) | Yes | Yes | `scripts/validate_federation.py` | -| MCP version pinning | Yes | Yes | `scripts/validate_federation.py` | -| Credential scan | Yes | Yes | `gitleaks` via script | -| AI agent compatibility | Yes | No | Manual review | -| Security scan (LLM) | On-demand | No | Maintainer-triggered | +- [FEDERATION_REQUEST_GUIDE.md](FEDERATION_REQUEST_GUIDE.md) · [SKILL_DESIGN_PRINCIPLES.md](SKILL_DESIGN_PRINCIPLES.md) · [COLLECTION_SPEC.md](COLLECTION_SPEC.md) · [Lola](https://github.com/LobsterTrap/lola)