diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3faaa2e..c2c5cd9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,86 +1,56 @@ -name: CI - -on: - push: - branches: [ main ] - pull_request: - -permissions: - contents: read - -concurrency: - group: ci-${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -jobs: - test: - name: Python ${{ matrix.python-version }} / ${{ matrix.os }} - runs-on: ${{ matrix.os }} - timeout-minutes: 20 - strategy: - fail-fast: false - matrix: - os: [ubuntu-latest, windows-latest] - python-version: ['3.12'] - - steps: - - uses: actions/checkout@v7 - - - name: Set up Python - uses: actions/setup-python@v6 - with: - python-version: ${{ matrix.python-version }} - cache: pip - - - name: Install dependencies - run: python -m pip install -r requirements.txt - - - name: Verify dependency consistency - run: python -m pip check - - - name: Contract compatibility (pinned cas-contracts v1.1) - # Consumer-side gate: fails red if autogen's pinned CAS contract version - # or the vendored v1.1 schema release drifts. See - # tests/test_contract_compatibility.py for the validated contract surface. - run: python -m pytest tests/test_contract_compatibility.py -q --tb=short - - - name: Run full test suite - run: | - python -m pytest -q --tb=short --cov --cov-branch --cov-report=xml --cov-fail-under=73 - - - name: Emit branch coverage telemetry - if: always() - shell: python - run: | - import json - import sys - import xml.etree.ElementTree as ET - from pathlib import Path - - required = 53.5 - coverage_file = Path("coverage.xml") - if not coverage_file.exists(): - print(json.dumps({ - "event": "ci_failure", - "metric": "branch-rate", - "coverage_percent": None, - "required": required, - "reason": "coverage.xml missing", - })) - sys.exit(1) - - branch_rate = float(ET.parse(coverage_file).getroot().attrib["branch-rate"]) * 100.0 - passed = branch_rate >= required - print(json.dumps({ - "event": "ci_pass" if passed else "ci_failure", - "metric": "branch-rate", - "coverage_percent": round(branch_rate, 2), - "required": required, - })) - sys.exit(0 if passed else 1) - - - name: Compile Python sources - run: python -m compileall autogen_starter autogen_dashboard maf_starter main.py -q - - - name: Validate dashboard JavaScript - run: node --check autogen_dashboard/static/app.js +name: CI + +on: + push: + branches: [ main ] + pull_request: + +permissions: + contents: read + +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + name: Python ${{ matrix.python-version }} / ${{ matrix.os }} + runs-on: ${{ matrix.os }} + timeout-minutes: 20 + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + python-version: ['3.12'] + + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + + - name: Set up Python + uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 + with: + python-version: ${{ matrix.python-version }} + cache: pip + + - name: Install dependencies + run: python -m pip install -r requirements.txt + + - name: Verify dependency consistency + run: python -m pip check + + - name: Contract compatibility (pinned cas-contracts v1.1) + # Consumer-side gate: fails red if autogen's pinned CAS contract version + # or the vendored v1.1 schema release drifts. See + # tests/test_contract_compatibility.py for the validated contract surface. + run: python -m pytest tests/test_contract_compatibility.py -q --tb=short + + - name: Run full test suite + run: | + python -m pip install pytest-cov + python -m pytest -q --tb=short --cov=. --cov-report=xml + + - name: Compile Python sources + run: python -m compileall autogen_starter autogen_dashboard maf_starter main.py -q + + - name: Validate dashboard JavaScript + run: node --check autogen_dashboard/static/app.js diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 157cc00..5824f1a 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -19,12 +19,12 @@ jobs: language: [ 'javascript', 'python' ] steps: - name: Checkout repository - uses: actions/checkout@v7 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 - name: Initialize CodeQL - uses: github/codeql-action/init@v4 + uses: github/codeql-action/init@54f647b7e1bb85c95cddabcd46b0c578ec92bc1a # v4 with: languages: ${{ matrix.language }} - name: Autobuild - uses: github/codeql-action/autobuild@v4 + uses: github/codeql-action/autobuild@54f647b7e1bb85c95cddabcd46b0c578ec92bc1a # v4 - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v4 + uses: github/codeql-action/analyze@54f647b7e1bb85c95cddabcd46b0c578ec92bc1a # v4 diff --git a/.github/workflows/pages.yml b/.github/workflows/pages.yml index c562f18..f904732 100644 --- a/.github/workflows/pages.yml +++ b/.github/workflows/pages.yml @@ -14,13 +14,13 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 15 steps: - - uses: actions/checkout@v7 - - uses: actions/setup-python@v6 + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 with: python-version: 3.x - run: pip install mkdocs-material - run: mkdocs build - - uses: actions/upload-pages-artifact@v5 + - uses: actions/upload-pages-artifact@fc324d3547104276b827a68afc52ff2a11cc49c9 # v5 with: path: ./site deploy: @@ -36,4 +36,4 @@ jobs: steps: - name: Deploy to GitHub Pages id: deployment - uses: actions/deploy-pages@v5 + uses: actions/deploy-pages@cd2ce8fcbc39b97be8ca5fce6e763baed58fa128 # v5 diff --git a/.github/workflows/pr-lint.yml b/.github/workflows/pr-lint.yml index ebc13b5..c515ec8 100644 --- a/.github/workflows/pr-lint.yml +++ b/.github/workflows/pr-lint.yml @@ -13,6 +13,6 @@ jobs: permissions: pull-requests: read steps: - - uses: amannn/action-semantic-pull-request@v6 + - uses: amannn/action-semantic-pull-request@48f256284bd46cdaab1048c3721360e808335d50 # v6 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index 84a62fa..67a151a 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -11,7 +11,7 @@ jobs: issues: write pull-requests: write steps: - - uses: actions/stale@v10 + - uses: actions/stale@eb5cf3af3ac0a1aa4c9c45633dd1ae542a27a899 # v10 with: days-before-stale: 60 days-before-close: 7 diff --git a/README.md b/README.md index 65ca9db..6fb4240 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,16 @@ This repo already carries more engineering evidence than the old README surfaced - `tests/test_phase5_ui_contract.py` and `tests/test_phase5_operator_views.py` lock the operator UI to timeline, routing, artifact, and specialist-view contracts. - `.github/workflows/ci.yml` installs the declared environment and runs the full suite, Python compilation, dependency consistency, and JavaScript syntax checks on Windows and Linux. +### Test Coverage + +`main`'s `Run full test suite` CI step installs `pytest-cov` and runs `pytest --cov=. --cov-report=xml`, +but does not currently fail the build on a coverage threshold. A ratcheted coverage gate is +**in progress (PR #11)**: it adds a `.coveragerc` with `branch = true` and a `--cov-fail-under` +threshold, plus branch-coverage telemetry read from the produced `coverage.xml`. Per the PR's +own recorded validation, the repo-local suite reached ~73.3% total coverage and ~54.6% branch +coverage against the ratcheted threshold. Until PR #11 merges, coverage is measured but not +enforced on `main`. + ## Quickstart The checked-in snapshot supports a clean-clone local dashboard and full validation workflow: @@ -69,6 +79,16 @@ Copy-Item .env.example .env .\.venv\Scripts\python.exe main.py dashboard --host 127.0.0.1 --port 8000 ``` +Do not install into the system Python on WSL/Linux. That interpreter may be externally managed +under PEP 668, which blocks direct `pip install` runs and creates ambiguous test environments. +Always use the repo-local `.venv` so verification and coverage come from `requirements.txt`, not +ambient machine packages. + +MAF 1.0 direction: the current operator shell still uses the local dashboard, but the +orchestration core is intentionally aligned with Microsoft Agent Framework's graph-style +workflow model. The next durable UI/runtime step is to expose the existing manager-led flow +through DevUI or AG-UI style surfaces rather than inventing a separate orchestration concept. + Run the complete regression suite before changing runtime behavior: ```powershell @@ -135,3 +155,5 @@ That is the right foundation for a later Azure-hosted control plane or worker bo ## License MIT -- see [LICENSE](LICENSE) + + diff --git a/docs/wiki/Architecture.md b/docs/wiki/Architecture.md new file mode 100644 index 0000000..598931c --- /dev/null +++ b/docs/wiki/Architecture.md @@ -0,0 +1,80 @@ +# Architecture + +## Worker fan-out + telemetry boundary + critic gate + +```mermaid +flowchart TD + subgraph Manager["Manager-led workflow (entities/repo_team/workflow.py)"] + Plan[Planner
gemini-2.5-pro] --> Research[Researcher
gemini-2.5-flash] + Research --> Impl[Implementer
gemini-2.5-pro] + Impl --> Review[Reviewer
gemini-2.5-pro] + end + subgraph Boundary["Worker boundary (maf_starter/worker_boundary.py)"] + WB[WorkerBoundary
async dispatch, run_id, status polling] + end + subgraph Telemetry["Telemetry (in progress, PR #12)"] + T[emit_failure_telemetry
structured JSON on stderr] + end + subgraph Critic["Peer critic gate (in progress, PR #14)"] + C[Deterministic pattern-scan
engine] + end + subgraph Fallback["Provider fallback (maf_starter/provider_fallback.py)"] + F[Gemini -> Anthropic -> local CLI] + end + Manager --> Boundary + Impl --> Fallback + Fallback -.->|on failure| Telemetry + Review --> Critic + Critic -.->|gates| Manager +``` + + + +## Worker fan-out (landed on `main`) + +`entities/repo_team/workflow.py` wires the canonical `planning -> research -> implementation -> +review -> validation` sequence via `agent_framework_orchestrations.SequentialBuilder`. Each +specialist is built in `maf_starter/team_factory.py` with a distinct model tier. Long-running +executions are dispatched through `WorkerBoundary` (`maf_starter/worker_boundary.py`), which +returns a `run_id` immediately and exposes `pending` / `running` / `done` / `error:` +status polling instead of blocking HTTP ingress on the full execution path. + +This matches the direction Microsoft is now pushing more explicitly in Agent Framework 1.0: +workflow-native orchestration with richer graphical operator surfaces instead of a single opaque +chat loop. Today this repo uses a sequential builder plus a custom dashboard. The intended next +step is to keep the current specialist topology but surface it through first-party MAF UI +primitives such as DevUI or AG-UI endpoints when the repo is ready. + +## Telemetry boundary — in progress (PR #12) + +`main` today does not have a `maf_starter/telemetry.py` module. PR #12 +(`feat(28-02): structured JSON failure telemetry + CLI fallback size guards`) adds +`emit_failure_telemetry(event, **fields)` — a stdlib-only, never-raising function that writes +one flushed JSON line to stderr — wired into `provider_fallback.py`'s fallback middleware at +`provider_failed`, `fallback_step_failed`, `fallback_succeeded`, and `fallback_exhausted` +points, plus a 1MB CLI output/prompt size guard. Until merged, provider-fallback failures are +still caught (existing `except Exception` boundaries in `provider_fallback.py` and +`worker_boundary.py`) but not emitted as structured telemetry. + +## Critic gate — in progress (PR #14) + +No critic module exists on `main` yet. PR #14 +(`feat(29-01): deterministic peer critic pattern-scan engine`) introduces a deterministic +pattern-scan reviewer intended to sit after the Reviewer specialist as an additional gate. +Until merged, review is limited to the LLM-based Reviewer agent's own assessment. + +## Provider routing and fallback (landed on `main`) + +`maf_starter/routing_policy.py` selects a model tier by task depth; `maf_starter/ +provider_fallback.py` retries across the Gemini API, optional Anthropic API, and local CLI +providers (`gemini.cmd`, `claude`, `codex.cmd`) on heuristic quota/rate-limit errors, recording +route-attempt history. + +## Approvals and bounded repo tools (landed on `main`) + +`maf_starter/tools.py` enforces repo-root path boundaries and blocks writes to sensitive +targets (e.g. `.env`). `maf_starter/approval_policy.py` classifies file operations and +validation commands so destructive or externally visible actions pause for operator approval +via the dashboard's approval surface. + + diff --git a/docs/wiki/Decisions.md b/docs/wiki/Decisions.md new file mode 100644 index 0000000..8a4394a --- /dev/null +++ b/docs/wiki/Decisions.md @@ -0,0 +1,40 @@ +# Decisions + +## ADR convention + +`docs/adr/README.md` establishes the convention (sequential numbering, Context/Decision/ +Consequences) but **no numbered ADR files exist in the repo yet**. Decisions to date live in +`.planning/phases/` plan/summary pairs instead. + +## Phase history (`.planning/phases/`, this repo's own GSD project) + +| Phase | Topic | +|---|---| +| 01 | Workspace and durable run foundation | +| 02 | Manager-led orchestration core | +| 03 | Specialist delegation and routing visibility | +| 04 | Autonomous repo execution and validation guardrails | +| 05 | Polished operator workbench | +| 06 | API boundary and control-plane contract | +| 07 | Worker boundary | + +See each `.planning/phases//*-SUMMARY.md` for the detailed record. + +## Open decisions tracked in this Phase 36 refresh + +- **PR #11** (`feat/phase-26-coverage-gates`) — already merged to `main`; it is now the + compatibility baseline that exposed the remaining stale branch stack. +- **PRs #12, #13, #14, #15, #16** — still open on GitHub but stale against current `main`. + Their surviving runtime, CI, and docs changes are being consolidated into one refreshed branch + instead of re-merging the old dependency snapshot piecemeal. + +## Directional decision + +The medium-term architecture should adopt Microsoft Agent Framework's first-party workflow and UI +direction where it fits the local-first product: + +- keep manager-led orchestration as a graph/workflow problem, not a free-form chat problem; +- prefer MAF-native workflow builders and DevUI or AG-UI style surfaces over inventing a parallel UI abstraction; +- keep the current dashboard only as a bridge while the richer graphical workflow surface matures. + + diff --git a/docs/wiki/Home.md b/docs/wiki/Home.md new file mode 100644 index 0000000..71f4530 --- /dev/null +++ b/docs/wiki/Home.md @@ -0,0 +1,40 @@ +# autogen Wiki + +## Role in the CAS portfolio + +`autogen` is the **Execution plane** of the Coding-Autopilot-System three-plane model (Control +/ Execution / Governance). Built on Microsoft Agent Framework, it runs manager-led, +specialist-delegated engineering work against a real repository: planning, research, +implementation, review, and validation, with bounded repo tools and an approval gate for +destructive actions. + +| Plane | This repo's responsibility | +|---|---| +| Control | *(not this repo — see `gsd-orchestrator`)* | +| Execution | Manager-led worker fan-out, bounded repo tools, provider routing/fallback, run artifacts | +| Governance | *(not this repo — see `Promptimprover`, `cas-contracts`, `cas-evals`)* | + +## Quickstart + +- [README.md](../../README.md) — Quickstart, configuration reference, evidence posture +- [Architecture](./Architecture.md) — worker fan-out, telemetry boundary (in progress), critic gate (in progress) +- [Operations](./Operations.md) — verified run/test/CI commands +- [Decisions](./Decisions.md) — phase history and open PRs + +## MAF 1.0 alignment + +The repo direction is to reuse Microsoft Agent Framework's workflow and UI model, not to fork it. + +- keep the current manager-led specialist graph and evolve it with official MAF workflow builders; +- keep the local operator experience, but converge toward DevUI or AG-UI style surfaces for richer execution traces and graphical workflow visibility; +- treat the legacy dashboard as a transition shell, not the long-term orchestration abstraction. + +## Ecosystem links + +Part of the [Coding-Autopilot-System](https://github.com/Coding-Autopilot-System) org: +[gsd-orchestrator](https://github.com/Coding-Autopilot-System/gsd-orchestrator) (control plane) · +[Promptimprover](https://github.com/Coding-Autopilot-System/Promptimprover) (prompt governance) · +[cas-contracts](https://github.com/Coding-Autopilot-System/cas-contracts) (shared schemas) · +[cas-evals](https://github.com/Coding-Autopilot-System/cas-evals) (evidence gate) + + diff --git a/docs/wiki/Operations.md b/docs/wiki/Operations.md new file mode 100644 index 0000000..c31768f --- /dev/null +++ b/docs/wiki/Operations.md @@ -0,0 +1,50 @@ +# Operations + +## Setup + +```powershell +git clone https://github.com/Coding-Autopilot-System/autogen.git +Set-Location autogen +python -m venv .venv +.\.venv\Scripts\python.exe -m pip install -r requirements.txt +Copy-Item .env.example .env +``` + +If you are running from WSL/Linux, do not install into the system interpreter. This repo expects +an isolated `.venv`; distro Python can be PEP 668 managed and will either reject direct package +installs or produce non-reproducible verification results. + +## Run + +```powershell +.\.venv\Scripts\python.exe main.py providers +.\.venv\Scripts\python.exe main.py dashboard --host 127.0.0.1 --port 8000 +``` + +## Test + +```powershell +.\.venv\Scripts\python.exe -m pytest -q --tb=short +``` + +Contract-compatibility gate only (consumer-side check against the pinned `cas-contracts` v1.1 +release): + +```powershell +.\.venv\Scripts\python.exe -m pytest tests/test_contract_compatibility.py -q --tb=short +``` + +## CI (`.github/workflows/ci.yml`, matrix: ubuntu-latest + windows-latest, Python 3.12, 20-minute timeout) + +1. `actions/checkout` pinned to a full commit SHA +2. `actions/setup-python` pinned to a full commit SHA (Python 3.12, pip cache) +3. `pip install -r requirements.txt` +4. `pip check` — dependency consistency +5. **Contract compatibility** — `tests/test_contract_compatibility.py`, fails red on pinned-contract drift +6. **Run full test suite** — installs `pytest-cov`, runs `pytest --cov=. --cov-report=xml` (coverage is measured; no `--cov-fail-under` threshold is enforced on `main` yet — see [Architecture](./Architecture.md) and PR #11) +7. `python -m compileall autogen_starter autogen_dashboard maf_starter main.py -q` +8. `node --check autogen_dashboard/static/app.js` — legacy dashboard JS syntax check + +A separate `.github/workflows/codeql.yml` runs CodeQL analysis (badge in the root `README.md`). + + diff --git a/maf_starter/critic.py b/maf_starter/critic.py new file mode 100644 index 0000000..a764a95 --- /dev/null +++ b/maf_starter/critic.py @@ -0,0 +1,157 @@ +from __future__ import annotations + +from dataclasses import dataclass +import re +import sys +from typing import Literal + +from maf_starter.config import Settings + + +@dataclass(frozen=True) +class Finding: + check: str + severity: Literal["blocking", "advisory"] + file: str + line: int | None + detail: str + + +@dataclass(frozen=True) +class DiffFile: + path: str + added_lines: list[str] + + +@dataclass(frozen=True) +class CriticReport: + findings: list[Finding] + blocking_count: int + advisory_count: int + exit_code: int + + +_BARE_EXCEPT_PATTERN = re.compile(r"^\s*except\s*:\s*$") +_EXCEPTION_EXCEPT_PATTERN = re.compile(r"^\s*except\s+Exception(?:\s+as\s+\w+)?\s*:\s*$") +_LOG_TOKENS = ("logger.", "logging.", "print(", "emit_failure_telemetry(", "telemetry.") +_TYPED_FAILURE_TOKENS = ("FailureState", "McpException", "ValueError", "RuntimeError", "InvalidOperationException") + + +def parse_unified_diff(diff_text: str) -> list[DiffFile]: + files: list[DiffFile] = [] + current_path: str | None = None + current_added: list[str] = [] + + for line in diff_text.splitlines(): + if line.startswith("diff --git "): + if current_path is not None: + files.append(DiffFile(path=current_path, added_lines=current_added)) + current_path = None + current_added = [] + continue + if line.startswith("+++ b/"): + current_path = line[6:] + continue + if current_path is None: + continue + if line.startswith("+") and not line.startswith("+++"): + current_added.append(line[1:]) + + if current_path is not None: + files.append(DiffFile(path=current_path, added_lines=current_added)) + + return files + + +def check_bare_except(file_path: str, added_lines: list[str]) -> list[Finding]: + findings: list[Finding] = [] + for index, line in enumerate(added_lines): + if not (_BARE_EXCEPT_PATTERN.match(line) or _EXCEPTION_EXCEPT_PATTERN.match(line)): + continue + window = added_lines[index + 1 : index + 5] + has_reraise = any("raise" in candidate for candidate in window) + has_logging = any(any(token in candidate for token in _LOG_TOKENS) for candidate in window) + has_typed_failure = any(any(token in candidate for token in _TYPED_FAILURE_TOKENS) for candidate in window) + if has_reraise and has_logging and has_typed_failure: + continue + findings.append( + Finding( + check="bare-except", + severity="blocking", + file=file_path, + line=index + 1, + detail="Broad exception handler added without a typed, logged re-raise path.", + ) + ) + return findings + + +def check_missing_telemetry(file_path: str, added_lines: list[str]) -> list[Finding]: + findings: list[Finding] = [] + for index, line in enumerate(added_lines): + if "except" not in line: + continue + window = added_lines[index + 1 : index + 5] + has_logging = any(any(token in candidate for token in _LOG_TOKENS) for candidate in window) + if has_logging: + continue + findings.append( + Finding( + check="missing-telemetry", + severity="advisory", + file=file_path, + line=index + 1, + detail="Exception path does not emit structured telemetry or logging.", + ) + ) + return findings + + +def check_file_size_limit(file_path: str, added_line_count: int, limit: int = 400) -> list[Finding]: + if added_line_count <= limit: + return [] + return [ + Finding( + check="file-size-limit", + severity="advisory", + file=file_path, + line=None, + detail=f"Added line count {added_line_count} exceeds limit {limit}.", + ) + ] + + +def run_critic(diff_text: str, *, llm_pass: bool = False, settings: Settings | None = None) -> CriticReport: + if llm_pass: + raise NotImplementedError("LLM critic pass is reserved for a future phase.") + + try: + _ = settings + files = parse_unified_diff(diff_text) + if not files: + raise ValueError("No unified diff content was found.") + + findings: list[Finding] = [] + for diff_file in files: + findings.extend(check_bare_except(diff_file.path, diff_file.added_lines)) + findings.extend(check_missing_telemetry(diff_file.path, diff_file.added_lines)) + findings.extend(check_file_size_limit(diff_file.path, len(diff_file.added_lines))) + + blocking_count = sum(1 for finding in findings if finding.severity == "blocking") + advisory_count = sum(1 for finding in findings if finding.severity == "advisory") + return CriticReport( + findings=findings, + blocking_count=blocking_count, + advisory_count=advisory_count, + exit_code=1 if blocking_count > 0 else 0, + ) + except Exception as exc: # noqa: BLE001 + print(f"critic error: {exc}", file=sys.stderr) + finding = Finding( + check="critic-error", + severity="advisory", + file="", + line=None, + detail=str(exc), + ) + return CriticReport(findings=[finding], blocking_count=0, advisory_count=1, exit_code=0) diff --git a/maf_starter/critic_cli.py b/maf_starter/critic_cli.py new file mode 100644 index 0000000..b984d81 --- /dev/null +++ b/maf_starter/critic_cli.py @@ -0,0 +1,46 @@ +from __future__ import annotations + +import argparse +from pathlib import Path +import sys + +from maf_starter.critic import run_critic + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(prog="critic", description="Run the Resilience First peer critic against a unified diff") + parser.add_argument("--diff", required=True, help="Diff file path or '-' to read from stdin") + parser.add_argument( + "--severity-gate", + choices=("blocking", "advisory"), + default="blocking", + help="Exit non-zero on blocking findings only, or on any finding in advisory mode", + ) + return parser + + +def main(argv: list[str] | None = None) -> int: + parser = build_parser() + args = parser.parse_args(argv) + + try: + if args.diff == "-": + diff_text = sys.stdin.read() + else: + diff_text = Path(args.diff).read_text(encoding="utf-8", errors="replace") + except (FileNotFoundError, OSError) as exc: + print(f"critic: unable to read diff: {exc}", file=sys.stderr) + return 2 + + report = run_critic(diff_text) + for finding in report.findings: + print(f"{finding.file}:{finding.line or '-'} [{finding.severity}] {finding.check} - {finding.detail}") + print(f"critic: {report.blocking_count} blocking, {report.advisory_count} advisory") + + if args.severity_gate == "advisory": + return 1 if (report.blocking_count + report.advisory_count) > 0 else 0 + return report.exit_code + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/maf_starter/provider_fallback.py b/maf_starter/provider_fallback.py index 758b1ec..7d31929 100644 --- a/maf_starter/provider_fallback.py +++ b/maf_starter/provider_fallback.py @@ -68,9 +68,12 @@ async def get_final_response(self): from maf_starter.execution_profile import LOCAL_PROFILE, ExecutionProfile from maf_starter.routing_policy import RoutingPlan, build_routing_plan from maf_starter.routing_types import CapabilityChange, ChainStep, RouteAttempt +from maf_starter.telemetry import emit_failure_telemetry FALLBACK_NOTICE = "[Fallback provider used because the earlier model/provider in the chain failed.]\n" +MAX_CLI_OUTPUT_BYTES = 1_000_000 +MAX_CLI_PROMPT_BYTES = 1_000_000 FALLBACK_ERROR_MARKERS = ( "resource_exhausted", "quota", @@ -132,6 +135,12 @@ async def fallback_middleware(context: ChatContext, call_next): error=last_error, ) ) + emit_failure_telemetry( + "provider_failed", + provider=route.primary_provider, + model=route.primary_model, + error=str(exc), + ) for step in route.fallback_steps: try: context.result = await _execute_chain_step( @@ -143,6 +152,12 @@ async def fallback_middleware(context: ChatContext, call_next): attempt_log=attempt_log, fallback_index=len(attempt_log), ) + emit_failure_telemetry( + "fallback_succeeded", + provider=step.provider, + model=step.model or step.label, + primary_error=str(last_error), + ) return except Exception as fallback_exc: last_error = fallback_exc @@ -156,8 +171,21 @@ async def fallback_middleware(context: ChatContext, call_next): error=fallback_exc, ) ) + emit_failure_telemetry( + "fallback_step_failed", + provider=step.provider, + model=step.model or step.label, + error=str(fallback_exc), + fallback_index=len(attempt_log) - 1, + ) continue + emit_failure_telemetry( + "fallback_exhausted", + primary_provider=route.primary_provider, + attempted_providers=[attempt.provider for attempt in attempt_log], + final_error=str(last_error), + ) raise last_error finally: reset_run_scope(scope_tokens) @@ -233,6 +261,12 @@ async def _stream(): error=last_error, ) ) + emit_failure_telemetry( + "provider_failed", + provider=route.primary_provider, + model=route.primary_model, + error=str(exc), + ) for step in route.fallback_steps: try: fallback_result = await _execute_chain_step( @@ -244,6 +278,12 @@ async def _stream(): attempt_log=attempt_log, fallback_index=len(attempt_log), ) + emit_failure_telemetry( + "fallback_succeeded", + provider=step.provider, + model=step.model or step.label, + primary_error=str(last_error), + ) if isinstance(fallback_result, ResponseStream): async for update in fallback_result: yield update @@ -274,6 +314,19 @@ async def _stream(): error=fallback_exc, ) ) + emit_failure_telemetry( + "fallback_step_failed", + provider=step.provider, + model=step.model or step.label, + error=str(fallback_exc), + fallback_index=len(attempt_log) - 1, + ) + emit_failure_telemetry( + "fallback_exhausted", + primary_provider=route.primary_provider, + attempted_providers=[attempt.provider for attempt in attempt_log], + final_error=str(last_error), + ) raise last_error if isinstance(original_stream, ResponseStream): @@ -791,6 +844,9 @@ def _run_subprocess( raise RuntimeError(f"{provider_name} failed: {detail}") output = completed.stdout.strip() + output_bytes = len(output.encode("utf-8", errors="replace")) + if output_bytes > MAX_CLI_OUTPUT_BYTES: + raise RuntimeError(f"{provider_name} output exceeds {MAX_CLI_OUTPUT_BYTES} bytes") if not output: raise RuntimeError(f"{provider_name} returned empty output") @@ -818,7 +874,10 @@ def _messages_to_prompt(messages: list[Message] | tuple[Message, ...]) -> str: for message in messages: rendered.append(f"{str(message.role).upper()}: {_message_text(message)}") rendered.append("ASSISTANT:") - return "\n\n".join(rendered) + prompt = "\n\n".join(rendered) + if len(prompt.encode("utf-8", errors="replace")) > MAX_CLI_PROMPT_BYTES: + raise ValueError(f"Rendered CLI prompt exceeds {MAX_CLI_PROMPT_BYTES} bytes") + return prompt def _message_text(message: Message) -> str: diff --git a/maf_starter/telemetry.py b/maf_starter/telemetry.py new file mode 100644 index 0000000..32fb740 --- /dev/null +++ b/maf_starter/telemetry.py @@ -0,0 +1,19 @@ +from __future__ import annotations + +import json +import sys +from datetime import datetime, timezone +from typing import Any + + +def emit_failure_telemetry(event: str, **fields: Any) -> None: + payload = { + "event": event, + "timestamp": datetime.now(timezone.utc).isoformat(), + **fields, + } + try: + sys.stderr.write(json.dumps(payload, separators=(",", ":")) + "\n") + sys.stderr.flush() + except Exception: # pragma: no cover - telemetry must never mask the original failure + return diff --git a/maf_starter/worker_boundary.py b/maf_starter/worker_boundary.py index 778bb91..1717618 100644 --- a/maf_starter/worker_boundary.py +++ b/maf_starter/worker_boundary.py @@ -4,6 +4,8 @@ from enum import Enum from typing import Any, Awaitable, Callable +from maf_starter.telemetry import emit_failure_telemetry + class WorkerProfile(str, Enum): LOCAL = "local" @@ -58,6 +60,7 @@ async def _run(self, run_id: str, workflow: Callable[[], Awaitable[Any]]) -> Non self._status[run_id] = "done" except Exception as exc: # noqa: BLE001 self._status[run_id] = f"error:{exc}" + emit_failure_telemetry("worker_task_failed", run_id=run_id, error=str(exc)) def get_status(self, run_id: str) -> _RunStatus | None: """Return the current status string for *run_id*, or None if unknown.""" diff --git a/tests/test_critic.py b/tests/test_critic.py new file mode 100644 index 0000000..0860e8d --- /dev/null +++ b/tests/test_critic.py @@ -0,0 +1,189 @@ +from __future__ import annotations + +import io +from pathlib import Path +import textwrap + +import pytest + +from maf_starter.critic import ( + check_bare_except, + check_file_size_limit, + check_missing_telemetry, + parse_unified_diff, + run_critic, +) +from maf_starter.critic_cli import build_parser, main + + +DIRTY_DIFF = textwrap.dedent( + """\ + diff --git a/app.py b/app.py + +++ b/app.py + @@ + +try: + + do_work() + +except Exception: + + pass + """ +) + +DIRTY_DIFF_WITH_TELEMETRY_GAP = textwrap.dedent( + """\ + diff --git a/app.py b/app.py + +++ b/app.py + @@ + +try: + + do_work() + +except ValueError: + + recover() + """ +) + +CLEAN_DIFF = textwrap.dedent( + """\ + diff --git a/app.py b/app.py + +++ b/app.py + @@ + +try: + + do_work() + +except Exception as exc: + + logger.error("failed", exc_info=exc) + + raise RuntimeError("typed failure") from exc + """ +) + + +def test_parse_unified_diff_returns_changed_files_and_added_lines() -> None: + diff_text = textwrap.dedent( + """\ + diff --git a/app.py b/app.py + +++ b/app.py + @@ + +first + diff --git a/worker.py b/worker.py + +++ b/worker.py + @@ + +second + """ + ) + + files = parse_unified_diff(diff_text) + + assert len(files) == 2 + assert files[0].path == "app.py" + assert files[0].added_lines == ["first"] + assert files[1].path == "worker.py" + assert files[1].added_lines == ["second"] + + +def test_check_bare_except_blocks_untyped_unlogged_handler() -> None: + findings = check_bare_except("app.py", ["except Exception:", " pass"]) + + assert len(findings) == 1 + assert findings[0].check == "bare-except" + assert findings[0].severity == "blocking" + + +def test_check_bare_except_allows_logged_typed_reraise() -> None: + findings = check_bare_except( + "app.py", + [ + "except Exception as exc:", + ' logger.error("boom", exc_info=exc)', + ' raise RuntimeError("typed failure") from exc', + ], + ) + + assert findings == [] + + +def test_check_missing_telemetry_returns_advisory_only() -> None: + findings = check_missing_telemetry("app.py", ["except ValueError:", " recover()"]) + + assert len(findings) == 1 + assert findings[0].check == "missing-telemetry" + assert findings[0].severity == "advisory" + + +def test_check_file_size_limit_flags_large_files() -> None: + findings = check_file_size_limit("big.py", 401, limit=400) + + assert len(findings) == 1 + assert findings[0].check == "file-size-limit" + assert findings[0].severity == "advisory" + + +def test_run_critic_blocks_dirty_diff() -> None: + report = run_critic(DIRTY_DIFF) + + assert report.exit_code == 1 + assert report.blocking_count == 1 + assert any(finding.check == "bare-except" for finding in report.findings) + + +def test_run_critic_passes_clean_diff() -> None: + report = run_critic(CLEAN_DIFF) + + assert report.exit_code == 0 + assert report.blocking_count == 0 + assert report.advisory_count == 0 + + +def test_run_critic_returns_advisory_error_on_malformed_input() -> None: + report = run_critic("not a diff at all") + + assert report.exit_code == 0 + assert report.blocking_count == 0 + assert report.advisory_count == 1 + assert report.findings[0].check == "critic-error" + + +def test_build_parser_defaults_to_blocking_gate() -> None: + args = build_parser().parse_args(["--diff", "-"]) + + assert args.diff == "-" + assert args.severity_gate == "blocking" + + +def test_main_returns_zero_for_clean_diff_file(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + diff_path = tmp_path / "clean.diff" + diff_path.write_text(CLEAN_DIFF, encoding="utf-8") + + exit_code = main(["--diff", str(diff_path)]) + + out = capsys.readouterr().out + assert exit_code == 0 + assert "critic: 0 blocking, 0 advisory" in out + + +def test_main_returns_one_for_blocking_diff_file(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + diff_path = tmp_path / "dirty.diff" + diff_path.write_text(DIRTY_DIFF, encoding="utf-8") + + exit_code = main(["--diff", str(diff_path)]) + + out = capsys.readouterr().out + assert exit_code == 1 + assert "[blocking] bare-except" in out + + +def test_main_returns_two_for_missing_diff_path(capsys: pytest.CaptureFixture[str]) -> None: + exit_code = main(["--diff", "missing.diff"]) + + err = capsys.readouterr().err + assert exit_code == 2 + assert "unable to read diff" in err + + +def test_main_reads_diff_from_stdin_and_supports_advisory_gate( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + monkeypatch.setattr("sys.stdin", io.StringIO(DIRTY_DIFF_WITH_TELEMETRY_GAP)) + + exit_code = main(["--diff", "-", "--severity-gate", "advisory"]) + + out = capsys.readouterr().out + assert exit_code == 1 + assert "critic: 0 blocking, 1 advisory" in out diff --git a/tests/test_provider_fallback_telemetry.py b/tests/test_provider_fallback_telemetry.py new file mode 100644 index 0000000..e38f88d --- /dev/null +++ b/tests/test_provider_fallback_telemetry.py @@ -0,0 +1,179 @@ +from __future__ import annotations + +import contextlib +import io +import json +import subprocess +import unittest +import uuid +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch + +from agent_framework import ChatResponse, Message + +from maf_starter.config import Settings, load_settings +from maf_starter.provider_fallback import ( + _messages_to_prompt, + _run_subprocess, + build_fallback_middleware, +) +from maf_starter.routing_policy import RoutingPlan +from maf_starter.routing_types import ChainStep +from maf_starter.telemetry import emit_failure_telemetry + + +SCRATCH_ROOT = Path(__file__).resolve().parents[1] / ".tmp-tests" + + +class RepoScratchTestCase(unittest.TestCase): + def make_scratch_dir(self) -> Path: + path = SCRATCH_ROOT / uuid.uuid4().hex + path.mkdir(parents=True, exist_ok=False) + self.addCleanup(lambda: path.exists() and __import__("shutil").rmtree(path, ignore_errors=True)) + return path + + +class ProviderFallbackTelemetryTests(RepoScratchTestCase): + def _make_settings(self, root: Path) -> Settings: + entities = root / "entities" + repo = root / "repo" + entities.mkdir() + repo.mkdir() + env = { + "MAF_API_KEY": "test-key", + "MAF_REPO_ROOT": str(repo), + "MAF_ENTITIES_DIR": str(entities), + "MAF_FALLBACK_CHAIN": "gemini-cli:gemini-2.5-pro,claude-cli:claude-sonnet-4-6", + } + with patch.dict("os.environ", env, clear=False): + return load_settings(project_root=root, env_path=root / ".missing-env") + + @staticmethod + def _make_route_plan() -> RoutingPlan: + return RoutingPlan( + mode="auto", + route_lane="auto", + tier="standard", + rationale="test route", + primary_provider="gemini", + primary_model="gemini-2.5-pro", + requested_provider="gemini", + requested_model="gemini-2.5-pro", + fallback_steps=( + ChainStep("gemini-cli", "gemini-2.5-pro"), + ChainStep("claude-cli", "claude-sonnet-4-6"), + ), + ) + + @staticmethod + def _make_context() -> SimpleNamespace: + return SimpleNamespace( + messages=[Message(role="user", text="hello fallback")], + options={}, + kwargs={}, + metadata={}, + result=None, + ) + + def test_emit_failure_telemetry_writes_single_parseable_json_line(self) -> None: + stream = io.StringIO() + with contextlib.redirect_stderr(stream): + emit_failure_telemetry("provider_failed", provider="gemini", error="quota exceeded") + + lines = stream.getvalue().strip().splitlines() + self.assertEqual(len(lines), 1) + payload = json.loads(lines[0]) + self.assertEqual(payload["event"], "provider_failed") + self.assertEqual(payload["provider"], "gemini") + self.assertEqual(payload["error"], "quota exceeded") + self.assertIn("timestamp", payload) + + def test_emit_failure_telemetry_swallows_stderr_write_failures(self) -> None: + fake_stderr = SimpleNamespace( + write=lambda _: (_ for _ in ()).throw(RuntimeError("stderr failed")), + flush=lambda: None, + ) + with patch("sys.stderr", fake_stderr): + self.assertIsNone(emit_failure_telemetry("provider_failed", provider="gemini", error="boom")) + + def test_fallback_middleware_emits_primary_step_and_exhausted_events(self) -> None: + root = self.make_scratch_dir() + settings = self._make_settings(root) + middleware = build_fallback_middleware( + settings, + primary_provider="gemini", + primary_model="gemini-2.5-pro", + routing_mode="auto", + ) + context = self._make_context() + stderr = io.StringIO() + + async def call_next(): + raise RuntimeError("rate limit from primary") + + attempts = [ + RuntimeError("rate limit in gemini-cli fallback"), + RuntimeError("quota exhausted in claude fallback"), + ] + + async def fake_execute_chain_step(**_kwargs): + raise attempts.pop(0) + + with ( + patch("maf_starter.provider_fallback.build_routing_plan", return_value=self._make_route_plan()), + patch("maf_starter.provider_fallback._execute_chain_step", side_effect=fake_execute_chain_step), + contextlib.redirect_stderr(stderr), + ): + with self.assertRaisesRegex(RuntimeError, "quota exhausted in claude fallback"): + __import__("asyncio").run(middleware(context, call_next)) + + events = [json.loads(line)["event"] for line in stderr.getvalue().strip().splitlines()] + self.assertEqual(events, ["provider_failed", "fallback_step_failed", "fallback_step_failed", "fallback_exhausted"]) + + def test_fallback_middleware_emits_recovery_event_when_fallback_succeeds(self) -> None: + root = self.make_scratch_dir() + settings = self._make_settings(root) + middleware = build_fallback_middleware( + settings, + primary_provider="gemini", + primary_model="gemini-2.5-pro", + routing_mode="auto", + ) + context = self._make_context() + stderr = io.StringIO() + + async def call_next(): + raise RuntimeError("rate limit from primary") + + async def fake_execute_chain_step(**_kwargs): + return ChatResponse(messages=[Message(role="assistant", text="fallback ok")]) + + with ( + patch("maf_starter.provider_fallback.build_routing_plan", return_value=self._make_route_plan()), + patch("maf_starter.provider_fallback._execute_chain_step", side_effect=fake_execute_chain_step), + contextlib.redirect_stderr(stderr), + ): + __import__("asyncio").run(middleware(context, call_next)) + + payloads = [json.loads(line) for line in stderr.getvalue().strip().splitlines()] + self.assertEqual(payloads[0]["event"], "provider_failed") + self.assertEqual(payloads[1]["event"], "fallback_succeeded") + self.assertNotIn("fallback_exhausted", [payload["event"] for payload in payloads]) + self.assertIsInstance(context.result, ChatResponse) + + def test_run_subprocess_rejects_oversized_output(self) -> None: + completed = subprocess.CompletedProcess(args=["codex.cmd"], returncode=0, stdout="x" * 32, stderr="") + with ( + patch("maf_starter.provider_fallback.subprocess.run", return_value=completed), + patch("maf_starter.provider_fallback.shutil.which", return_value="codex.cmd"), + patch("maf_starter.provider_fallback.MAX_CLI_OUTPUT_BYTES", 8), + ): + with self.assertRaisesRegex(RuntimeError, "output exceeds 8 bytes"): + _run_subprocess("codex-cli", ["codex.cmd", "exec"], Path.cwd(), None) + + def test_messages_to_prompt_rejects_oversized_prompt(self) -> None: + messages = [Message(role="user", text="x" * 32)] + with patch("maf_starter.provider_fallback.MAX_CLI_PROMPT_BYTES", 8): + with self.assertRaisesRegex(ValueError, "Rendered CLI prompt exceeds 8 bytes"): + _messages_to_prompt(messages) diff --git a/tests/test_worker_boundary.py b/tests/test_worker_boundary.py index d0abe6d..5b251da 100644 --- a/tests/test_worker_boundary.py +++ b/tests/test_worker_boundary.py @@ -1,6 +1,9 @@ from __future__ import annotations import asyncio +import contextlib +import io +import json import unittest from maf_starter.worker_boundary import WorkerBoundary, WorkerProfile @@ -71,19 +74,25 @@ async def controlled_workflow(): async def test_status_records_error_on_exception(self) -> None: boundary = WorkerBoundary() + stderr = io.StringIO() async def failing_workflow(): raise ValueError("something went wrong") - boundary.submit_async("run-3", failing_workflow) - # Yield so the task can run and fail - await asyncio.sleep(0) + with contextlib.redirect_stderr(stderr): + boundary.submit_async("run-3", failing_workflow) + # Yield so the task can run and fail + await asyncio.sleep(0) status = boundary.get_status("run-3") assert status is not None self.assertTrue(status.startswith("error:"), f"Expected error: prefix, got: {status!r}") self.assertIn("something went wrong", status) self.assertTrue(boundary.is_done("run-3")) + payload = json.loads(stderr.getvalue().strip()) + self.assertEqual(payload["event"], "worker_task_failed") + self.assertEqual(payload["run_id"], "run-3") + self.assertEqual(payload["error"], "something went wrong") def test_get_status_returns_none_for_unknown_run(self) -> None: boundary = WorkerBoundary()