From 6819cee72486ee193d6e169c18cfe939581307d6 Mon Sep 17 00:00:00 2001 From: Subham Sangwan Date: Fri, 1 May 2026 23:24:47 +0530 Subject: [PATCH 1/2] feat: Breeze-aware AI agent skills system (GSoC 2026 PoC) --- .agents/skills/run-static-checks/SKILL.md | 89 +++++ .agents/skills/run-unit-tests/SKILL.md | 109 ++++++ .agents/skills/stage-changes/SKILL.md | 68 ++++ .pre-commit-config.yaml | 7 + AGENTS.md | 420 ++------------------- CONSTITUTION.md | 167 ++++++++ scripts/ci/prek/breeze_context.py | 252 +++++++++++++ scripts/ci/prek/validate_skills.py | 116 ++++++ scripts/tests/test_breeze_context.py | 95 +++++ scripts/tests/test_contributor_scenario.py | 75 ++++ scripts/tests/test_edge_cases.py | 50 +++ 11 files changed, 1062 insertions(+), 386 deletions(-) create mode 100644 .agents/skills/run-static-checks/SKILL.md create mode 100644 .agents/skills/run-unit-tests/SKILL.md create mode 100644 .agents/skills/stage-changes/SKILL.md create mode 100644 CONSTITUTION.md create mode 100644 scripts/ci/prek/breeze_context.py create mode 100755 scripts/ci/prek/validate_skills.py create mode 100644 scripts/tests/test_breeze_context.py create mode 100644 scripts/tests/test_contributor_scenario.py create mode 100644 scripts/tests/test_edge_cases.py diff --git a/.agents/skills/run-static-checks/SKILL.md b/.agents/skills/run-static-checks/SKILL.md new file mode 100644 index 0000000000000..e80cd537c45fd --- /dev/null +++ b/.agents/skills/run-static-checks/SKILL.md @@ -0,0 +1,89 @@ + + + +--- +name: run-static-checks +description: Run prek (pre-commit) static checks including ruff linting, formatting, and type checks on Airflow code. Works identically on host and inside Breeze. +--- + + +Run Static Checks +================= + +Run the Airflow pre-commit pipeline (`prek`) to lint, format, and type-check changed +files. This must pass before any PR is merged. + +Source of truth for this workflow is `contributing-docs` contributor guidance; the +commands below are the Breeze-aware execution mapping for agents. + +Context Detection +----------------- + +`prek` is installed both on the **host** and inside **Breeze**. The command is +identical in both environments — no context switching is needed. + +Commands +-------- + +```bash +# Run all checks on files changed since main (standard pre-PR flow) +prek run --from-ref main --stage pre-commit + +# Run slower manual checks too +prek run --from-ref main --stage manual + +# Run only ruff linting +prek run ruff --from-ref main + +# Run only ruff formatter +prek run ruff-format --from-ref main + +# Run a single check by hook ID (e.g. mypy) +prek run mypy --from-ref main +``` + +Workflow Context +---------------- + +This is **Scenario 1, Step 2** of the standard Airflow contributor workflow: + +1. stage-changes +2. → **run-static-checks** (this skill) +3. run-unit-tests + +Always run static checks **before** running unit tests to catch simple formatting +or import errors early. + +Prerequisites +------------- + +- Changes must be staged (`git add`) before running — see the `stage-changes` skill. +- `prek` must be installed: `uv tool install prek` +- Hooks must be enabled: `prek install` + +Interpreting Failures +--------------------- + +| Exit Code | Meaning | +|---|---| +| 0 | All checks passed | +| 1 | One or more checks failed — read the output to identify which hooks failed | + +When a check fails, `prek` prints the hook name and the specific files/lines that +failed. Fix the reported issues and re-run. Many hooks (ruff, ruff-format) auto-fix +on first run — check `git diff` after failure to see auto-fixes. + +Common Issues +------------- + +- **`ruff` lint error:** Fix manually based on the error message, then re-stage and retry. +- **`ruff-format` failure:** Run `uv run ruff format ` then re-stage and retry. +- **`mypy` type error:** Fix the type annotation issue flagged by the error message. +- **`check-xml` / other infra checks:** Usually auto-fixed by the hook itself. + +Success Criteria +---------------- + +`prek run --from-ref main --stage pre-commit` exits with code 0 and prints +`Passed` for every hook. diff --git a/.agents/skills/run-unit-tests/SKILL.md b/.agents/skills/run-unit-tests/SKILL.md new file mode 100644 index 0000000000000..a61ba4873283b --- /dev/null +++ b/.agents/skills/run-unit-tests/SKILL.md @@ -0,0 +1,109 @@ + + + +--- +name: run-unit-tests +description: Run Airflow unit tests with context-aware command selection. Uses uv on the host and breeze run inside the container. Supports targeted test paths to avoid running the full suite. +--- + + +Run Unit Tests +============== + +Run a targeted subset of Airflow unit tests. The correct command differs depending +on whether you are running on the **host** or inside a **Breeze** container. + +Source of truth for test workflow semantics is `contributing-docs`; this skill maps +that guidance into context-aware host/Breeze command execution. + +Context Detection +----------------- + +Check for the `BREEZE` environment variable before choosing a command: + +```bash +# You are inside Breeze if this variable is set: +echo $BREEZE +``` + +| Context | Detection | Command | +|---|---|---| +| **Host** | `BREEZE` absent | `uv run --project pytest ` | +| **Breeze** | `BREEZE` present | `breeze run pytest ` | + +You can also call `python scripts/ci/prek/breeze_context.py` to auto-detect and +print `host` or `breeze`. + +Commands +-------- + +On the Host +----------- + +`` is the folder containing the relevant `pyproject.toml`, e.g. +`airflow-core`, `providers/amazon`, `task-sdk`, `scripts`. + +```bash +# Run a single test method +uv run --project pytest path/to/test.py::TestClass::test_method -xvs + +# Run all tests in a file +uv run --project pytest path/to/test.py -xvs + +# Run all tests in a package +uv run --project pytest path/to/package/ -xvs + +# If uv fails due to missing system dependencies, fall back to Breeze: +breeze testing core-tests --run-in-parallel +``` + +Inside Breeze +------------- + +```bash +# Run a single test method +breeze run pytest path/to/test.py::TestClass::test_method -xvs + +# Run all tests in a file +breeze run pytest path/to/test.py -xvs + +# Run Breeze's built-in parallel test runners +breeze testing core-tests --run-in-parallel +breeze testing providers-tests --run-in-parallel +``` + +Workflow Context +---------------- + +This is **Scenario 2** of the standard Airflow contributor workflow: + +1. stage-changes +2. run-static-checks +3. → **run-unit-tests** (this skill) + +Run static checks first to avoid failing tests due to trivial formatting issues. + +Prerequisites +------------- + +- **Host:** `uv` must be installed and the project synced. +- **Breeze:** Docker must be running and the Breeze image built. In case breeze image needs rebuilding, it should be rebuilt (answer yes to the question of rebuilding). +- Tests requiring a database backend: use `--backend postgres` or `--backend mysql`. + +Interpreting Results +-------------------- + +| Exit Code | Meaning | +|---|---| +| 0 | All selected tests passed | +| 1 | One or more tests failed — check `FAILED` lines in output | + +Use `-xvs` flags for verbose output and early exit on first failure. This makes +it easier to read the root cause when tests fail. + +Success Criteria +---------------- + +`pytest` exits with code 0 and reports `passed` for all selected tests with no +`FAILED` or `ERROR` lines. diff --git a/.agents/skills/stage-changes/SKILL.md b/.agents/skills/stage-changes/SKILL.md new file mode 100644 index 0000000000000..50f301fcdf277 --- /dev/null +++ b/.agents/skills/stage-changes/SKILL.md @@ -0,0 +1,68 @@ + + + +--- +name: stage-changes +description: Stage changed files for commit using git add. This is a host-only operation — it must never run inside a Breeze container. +--- + + +Stage Changes +============= + +Stage specific files or directories before running static checks or committing. + +Source of truth for contribution workflow order is `contributing-docs`; this skill +enforces the host/container guardrails when applying that workflow. + +Host-Only Guardrail +------------------- + +This skill operates on the **host** working tree. Running `git` commands inside a +Breeze container will silently operate on container-local paths that differ from your +actual working tree. + +**Before running:** Confirm you are on the host by checking that the `BREEZE` +environment variable is **not** set. If it is set, you are inside Breeze — exit +first (`exit` or `Ctrl-D`), then stage changes. + +Commands +-------- + +```bash +# Stage a single file +git add path/to/file.py + +# Stage multiple files +git add path/to/file1.py path/to/file2.py + +# Stage all changes in a directory +git add providers/amazon/ + +# Stage all tracked changes +git add -u +``` + +Workflow Context +---------------- + +This is **Scenario 1, Step 1** of the standard Airflow contributor workflow: + +1. → **stage-changes** (this skill) +2. run-static-checks +3. run-unit-tests + +Always stage changes *before* running `prek` (pre-commit) so the hooks operate on +the correct set of files. + +Prerequisites +------------- + +- You must be on the **host** (not inside Breeze). See guardrail above. +- Files must already exist and be tracked by git (or explicitly added with `git add`). + +Success Criteria +---------------- + +`git status` shows staged changes under "Changes to be committed". diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index acf47e9adebb3..f8877c223fa55 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -438,6 +438,13 @@ repos: # changes quickly - especially when we want the early modifications from the first local group # to be applied before the non-local prek hooks are run hooks: + - id: validate-agent-skills + name: Validate Agent Skills structure + entry: ./scripts/ci/prek/validate_skills.py + language: python + pass_filenames: false + additional_dependencies: ['pyyaml>=6.0.3'] + files: ^\.agents/skills/.*$ - id: check-shared-distributions-structure name: Check shared distributions structure entry: ./scripts/ci/prek/check_shared_distributions_structure.py diff --git a/AGENTS.md b/AGENTS.md index 1fc6aed11514b..7c559bc52a602 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,409 +1,57 @@ - # AGENTS instructions -## Naming +## Purpose -Write **Dag** (title case) in all prose. Keep the all-caps or lowercase -spelling only when reproducing a literal code token — never rewrite these, -even inside fenced code blocks: +This file is a short map for agents. Load this first, then load CONSTITUTION.md for +project-wide modification rules and constraints. -- Python: the SDK class `DAG` (`from airflow.sdk import DAG`, - `dag = DAG("my_dag", ...)`); identifiers like `dag_id`, `dag`, `my_dag`. -- CLI: `airflow dags list`, `airflow dags test`, etc. -- Paths and config keys: `dag_processing/`, `dagprocessor`, `get_dag`, etc. -- Anti-pattern quotes that show the wrong form to teach the rule itself - (e.g., `Use "DAG" — always write "Dag"`). - -Don't spell out **Directed Acyclic Graph** except for historical context. - -## Environment Setup +## Main entrypoints - Install prek: `uv tool install prek` - Enable commit hooks: `prek install` - Install breeze shim (one-time, per machine): `scripts/tools/setup_breeze` — installs `~/.local/bin/breeze` that runs breeze via `uvx` from the current git worktree's `dev/breeze` (so each worktree, including ephemeral agent worktrees, gets its own breeze tied to its sources). See [ADR 0017](dev/breeze/doc/adr/0017-use-uvx-to-run-breeze-from-local-sources.md). -- **Never run pytest, python, or airflow commands directly on the host** — always use `breeze`. +- **Never run pytest, python, or airflow commands directly on the host** — prefer `breeze run` for integration tests. Unit tests can run on the host via `uv run`. - Place temporary scripts in `dev/` (mounted as `/opt/airflow/dev/` inside Breeze). +- **DevContainers & Worktrees:** DevContainers are treated as host environments (no `BREEZE` var). Worktrees are supported via the `breeze` shim. -## Commands - -`` is folder where pyproject.toml of the package you want to test is located. For example, `airflow-core` or `providers/amazon`. -`` is the branch the PR will be merged into — usually `main`, but could be `v3-1-test` when creating a PR for the 3.1 branch. - -- **Run a single test:** `uv run --project pytest path/to/test.py::TestClass::test_method -xvs` -- **Run a test file:** `uv run --project pytest path/to/test.py -xvs` -- **Run all tests in package:** `uv run --project pytest path/to/package -xvs` -- **If uv tests fail with missing system dependencies, run the tests with breeze**: `breeze run pytest -xvs` -- **Run a Python script:** `uv run --project python dev/my_script.py` -- **Run core or provider tests suite in parallel:** `breeze testing --run-in-parallel` (test groups: `core-tests`, `providers-tests`) -- **Run core or provider db tests suite in parallel:** `breeze testing --run-db-tests-only --run-in-parallel` (test groups: `core-tests`, `providers-tests`) -- **Run core or provider non-db tests suite in parallel:** `breeze testing --skip-db-tests --use-xdist` (test groups: `core-tests`, `providers-tests`) -- **Run single provider complete test suite:** `breeze testing providers-tests --test-type "Providers[PROVIDERS_LIST]"` (e.g., `Providers[google]` or `Providers[amazon]` or "Providers[amazon,google]") -- **Run Helm tests in parallel with xdist** `breeze testing helm-tests --use-xdist` -- **Run Helm tests with specific K8s version:** `breeze testing helm-tests --use-xdist --kubernetes-version 1.35.0` -- **Run specific Helm test type:** `breeze testing helm-tests --use-xdist --test-type ` (types: `airflow_aux`, `airflow_core`, `apiserver`, `dagprocessor`, `other`, `redis`, `security`, `statsd`, `webserver`) -- **Run other suites of tests** `breeze testing ` (test groups: `airflow-ctl-tests`, `docker-compose-tests`, `task-sdk-tests`) -- **Run scripts tests:** `uv run --project scripts pytest scripts/tests/ -xvs` -- **Run Airflow CLI:** `breeze run airflow dags list` -- **Type-check (non-providers):** run the prek hook — `prek run mypy- --all-files` (e.g. `mypy-airflow-core`, `mypy-task-sdk`, `mypy-shared-logging`; each `shared/` workspace member has its own `mypy-shared-` hook). The hook uses a dedicated virtualenv and mypy cache under `.build/mypy-venvs//` and `.build/mypy-caches//`; mypy itself is installed from `uv.lock` via the `mypy` dependency group (`uv sync --group mypy`), so it never mutates your project `.venv`. The hook prefers `uv` from the project's main `.venv/bin/uv` (installed by `uv sync` — `uv` is part of the `dev` dependency group via the `all` extras) for a project-pinned uv version; it falls back to `uv` on `$PATH` with a warning if that binary is missing. Clear with `breeze down --cleanup-mypy-cache`. -- **Type-check (providers):** `breeze run mypy path/to/code` -- **Lint with ruff only:** `prek run ruff --from-ref ` -- **Format with ruff only:** `prek run ruff-format --from-ref ` -- **Run regular (fast) static checks:** `prek run --from-ref --stage pre-commit` -- **Run manual (slower) checks:** `prek run --from-ref --stage manual` -- **Build docs:** `breeze build-docs` -- **Determine which tests to run based on changed files:** `breeze selective-checks --commit-ref ` - -SQLite is the default backend. Use `--backend postgres` or `--backend mysql` for integration tests that need those databases. If Docker networking fails, run `docker network prune`. - -## Repository Structure - -UV workspace monorepo. Key paths: - -- `airflow-core/src/airflow/` — core scheduler, API, CLI, models - - `models/` — SQLAlchemy models (DagModel, TaskInstance, DagRun, Asset, etc.) - - `jobs/` — scheduler, triggerer, Dag processor runners - - `api_fastapi/core_api/` — public REST API v2, UI endpoints - - `api_fastapi/execution_api/` — task execution communication API - - `dag_processing/` — Dag parsing and validation - - `cli/` — command-line interface - - `ui/` — React/TypeScript web interface (Vite) -- `task-sdk/` — lightweight SDK for Dag authoring and task execution runtime - - `src/airflow/sdk/execution_time/` — task runner, supervisor -- `providers/` — 100+ provider packages, each with its own `pyproject.toml` -- `airflow-ctl/` — management CLI tool -- `chart/` — Helm chart for Kubernetes deployment -- `dev/` — development utilities and scripts used to bootstrap the environment, releases, breeze dev env -- `scripts/` — utility scripts for CI, Docker, and prek hooks (workspace distribution `apache-airflow-scripts`) - - `ci/prek/` — prek (pre-commit) hook scripts; shared utilities in `common_prek_utils.py` - - `tests/` — pytest tests for the scripts; run with `uv run --project scripts pytest scripts/tests/` - -The `uv.lock` file is generated by `uv lock`, `uv sync` and is committed to the repo - it contains snapshot of -versions of all dependencies used for development of Airflow. If at any point in time you have a conflict -with `uv.lock`, simply delete it and run `uv lock` to regenerate it. - - -## Architecture Boundaries - -1. Users author Dags with the Task SDK (`airflow.sdk`). -2. Dag File Processor parses Dag files in separate processes and stores serialized Dags in the metadata DB. Software guards prevent individual parsing processes from accessing the database directly and enforce use of the Execution API, but these guards do not protect against intentional bypassing by malicious or misconfigured code. -3. Scheduler reads serialized Dags — **never runs user code** — and creates Dag runs / task instances. -4. Workers execute tasks via Task SDK and communicate with the API server through the Execution API — **never access the metadata DB directly**. Each task receives a short-lived JWT token scoped to its task instance ID. -5. API Server serves the React UI and handles all client-database interactions. -6. Triggerer evaluates deferred tasks/sensors in separate processes. Like the Dag File Processor, software guards steer it through the Execution API rather than direct database access, but these guards do not protect against intentional bypassing by malicious or misconfigured code. -7. Shared libraries that are symbolically linked to different Python distributions are in `shared` folder. -8. Airflow uses `uv workspace` feature to keep all the distributions sharing dependencies and venv -9. Each of the distributions should declare other needed distributions: `uv --project sync` command acts on the selected project in the monorepo with only dependencies that it has - -## Security Model - -When reviewing code, writing security documentation, or performing security research, keep in -mind the following aspects of Airflow's security model. The authoritative reference is -[`airflow-core/docs/security/security_model.rst`](airflow-core/docs/security/security_model.rst) -and [`airflow-core/docs/security/jwt_token_authentication.rst`](airflow-core/docs/security/jwt_token_authentication.rst). - -Particularly, the intentional design choices, that are not security vulnerabilities and should not be -reported as such are described in "What is NOT considered a security vulnerability" chapter of the security model. - -**When flagging security concerns, distinguish between:** - -1. **Actual vulnerabilities** — code that violates the documented security model (e.g., a worker - gaining database access it shouldn't have, a Scheduler executing user code, an unauthenticated - user accessing protected endpoints). -2. **Known limitations** — documented gaps where the current implementation doesn't provide full - isolation (e.g., DFP/Triggerer database access, shared Execution API resources, multi-team - not enforcing task-level isolation). These are tracked for improvement in future versions and - should not be reported as new findings. -3. **Deployment hardening opportunities** — measures a Deployment Manager can take to improve - isolation beyond what Airflow enforces natively (e.g., per-component configuration, asymmetric - JWT keys, network policies). These belong in deployment guidance, not as code-level issues. - -# Shared libraries - -- shared libraries provide implementation of some common utilities like logging, configuration where the code should be reused in different distributions (potentially in different versions) -- we have a number of shared libraries that are separate, small Python distributions located under `shared` folder -- each of the libraries has it's own src, tests, pyproject.toml and dependencies -- sources of those libraries are symbolically linked to the distributions that are using them (`airflow-core`, `task-sdk` for example) -- tests for the libraries (internal) are in the shared distribution's test and can be run from the shared distributions -- tests of the consumers using the shared libraries are present in the distributions that use the libraries and can be run from there - -## Coding Standards - -- **Always format and check Python files with ruff immediately after writing or editing them:** `uv run ruff format ` and `uv run ruff check --fix `. Do this for every Python file you create or modify, before moving on to the next step. -- No `assert` in production code. -- `time.monotonic()` for durations, not `time.time()`. -- In `airflow-core`, functions with a `session` parameter must not call `session.commit()`. Use keyword-only `session` parameters. -- Imports at top of file. Valid exceptions: circular imports, lazy loading for worker isolation, `TYPE_CHECKING` blocks. -- Guard heavy type-only imports (e.g., `kubernetes.client`) with `TYPE_CHECKING` in multi-process code paths. -- Define dedicated exception classes or use existing exceptions such as `ValueError` instead of raising the broad `AirflowException` directly. Each error case should have a specific exception type that conveys what went wrong. -- Apache License header on all new files (prek enforces this). -- Newsfragments are only used by distributions whose release process consumes them via towncrier — currently `airflow-core/newsfragments/`, `chart/newsfragments/`, and `dev/mypy/newsfragments/` — and only for major or breaking changes (usually coordinated during review; do not add by default). **Never add newsfragments for `providers/` or `airflow-ctl/`** — those distributions are released from `main` and their release managers regenerate the changelog from `git log`, so per-PR newsfragments are not consumed (see `dev/README_RELEASE_PROVIDERS.md` and `dev/README_RELEASE_AIRFLOWCTL.md`). For a user-visible note in those distributions, edit the changelog directly: `providers//docs/changelog.rst` for providers, `airflow-ctl/RELEASE_NOTES.rst` for airflow-ctl. Changes to `task-sdk/` ship in `airflow-core` — use `airflow-core/newsfragments/`. - -## Testing Standards - -- Add tests for new behavior — cover success, failure, and edge cases. -- Use pytest patterns, not `unittest.TestCase`. -- Use `spec`/`autospec` when mocking. -- Use `time_machine` for time-dependent tests. Do not use `datetime.now()` -- Use `@pytest.mark.parametrize` for multiple similar inputs. -- Use `@pytest.mark.db_test` for tests that require database access. -- Test fixtures: `devel-common/src/tests_common/pytest_plugin.py`. -- Test location mirrors source: `airflow/cli/cli_parser.py` → `tests/cli/test_cli_parser.py`. -- Do not use `caplog` in tests, prefer checking logic and not log output. - - -## Commits and PRs - -Write commit messages focused on user impact, not implementation details. - -- **Good:** `Fix airflow dags test command failure without serialized Dags` -- **Good:** `UI: Fix Grid view not refreshing after task actions` -- **Bad:** `Initialize Dag bundles in CLI get_dag function` - -For `airflow-core` (and `chart/`, `dev/mypy/`) user-visible changes, add a newsfragment in that distribution's `newsfragments/` directory: -`echo "Brief description" > airflow-core/newsfragments/{PR_NUMBER}.{bugfix|feature|improvement|doc|misc|significant}.rst` - -**Do not add newsfragments for `providers/` or `airflow-ctl/`** — their release managers regenerate the changelog from `git log` and do not consume newsfragments. Update the changelog directly when needed: `providers//docs/changelog.rst` (see `providers/AGENTS.md`) or `airflow-ctl/RELEASE_NOTES.rst`. Changes to `task-sdk/` use `airflow-core/newsfragments/` since task-sdk ships in airflow-core. - -- NEVER add Co-Authored-By with yourself as co-author of the commit. Agents cannot be authors, humans can be, Agents are assistants. - -### Git remote naming conventions - -Airflow standardises on two git remote names, and the rest of this file, the -contributing docs, and the release docs all assume them: - -- **`upstream`** — the canonical `apache/airflow` repository (fetch from here). -- **`origin`** — the contributor's fork of `apache/airflow` (push PR branches here). - -Always push branches to `origin`. Never push directly to `upstream` (and never -push directly to `main` on either remote). - -**Before running any remote-based command, run `git remote -v` and verify the -names match this convention.** If they do not — for example, the upstream remote -is called `apache`, or `origin` points at `apache/airflow` with the fork under a -different name like `fork` — **do not silently go along with the existing -names**. Surface the mismatch to the user and propose the exact rename commands -to bring the checkout in line with the convention, then ask the user to confirm -before running them. Examples: - -- Upstream is named `apache`, fork is `origin` (common legacy layout): - - ```bash - git remote rename apache upstream - ``` - -- `origin` points at `apache/airflow` and the fork is named `fork` (release-manager - / "cloned upstream directly" layout): - - ```bash - git remote rename origin upstream - git remote rename fork origin - ``` - -- Upstream is missing entirely: - - ```bash - git remote add upstream https://github.com/apache/airflow.git - # or, for SSH: - git remote add upstream git@github.com:apache/airflow.git - ``` - -- Fork is missing entirely: - - ```bash - gh repo fork apache/airflow --remote --remote-name origin - ``` - -After any rename/add, re-run `git remote -v` to confirm the new state before -continuing with commands that assume `upstream` / `origin`. - -If a doc, script, or command you're about to run uses the old `apache` name (or -any other variant), **translate it to the `upstream` convention** in what you -propose to the user, rather than perpetuating the old name. Flag the stale -documentation so it can be fixed in a follow-up. - -### Creating Pull Requests - -**Always push to the user's fork (`origin`)**, not to `upstream` (`apache/airflow`). -Never push directly to `main`. - -Before pushing, confirm the remote setup matches the conventions above -(`upstream` → `apache/airflow`, `origin` → your fork). Run `git remote -v` and, -if the names don't match, propose renames as described in "Git remote naming -conventions" — ask the user to confirm before running them. - -If the fork remote does not exist at all, create one: - -```bash -gh repo fork apache/airflow --remote --remote-name origin -``` - -Before pushing, perform a self-review of your changes following the Gen-AI review guidelines -in [`contributing-docs/05_pull_requests.rst`](contributing-docs/05_pull_requests.rst) and the -code review checklist in [`.github/instructions/code-review.instructions.md`](.github/instructions/code-review.instructions.md): - -1. Review the full diff (`git diff main...HEAD`) and verify every change is intentional and - related to the task — remove any unrelated changes. -2. Read `.github/instructions/code-review.instructions.md` and check your diff against every - rule — architecture boundaries, database correctness, code quality, testing requirements, - API correctness, and AI-generated code signals. Fix any violations before pushing. -3. Confirm the code follows the project's coding standards and architecture boundaries - described in this file. -4. Run regular (fast) static checks (`prek run --from-ref --stage pre-commit`) - and fix any failures. This includes mypy checks for non-provider projects (airflow-core, task-sdk, airflow-ctl, dev, scripts, devel-common). -5. Run manual (slower) checks (`prek run --from-ref --stage manual`) and fix any failures. -6. Run relevant individual tests and confirm they pass. -7. Find which tests to run for the changes with selective-checks and run those tests in parallel to confirm they pass and check for CI-specific issues. -8. Check for security issues — no secrets, no injection vulnerabilities, no unsafe patterns. - -Before pushing, always rebase your branch onto the latest target branch (usually `main`) -to avoid merge conflicts and ensure CI runs against up-to-date code: - -```bash -git fetch upstream -git rebase upstream/ -``` - -If there are conflicts, resolve them and continue the rebase. If the rebase is too complex, -ask the user for guidance. - -Then push the branch to your fork (`origin`) and open the PR creation page in the browser -with the body pre-filled (including the generative AI disclosure already checked): - -```bash -git push -u origin -gh pr create --web --title "Short title (under 70 chars)" --body "$(cat <<'EOF' -Brief description of the changes. - -closes: #ISSUE (if applicable) - ---- - -##### Was generative AI tooling used to co-author this PR? - -- [X] Yes — - -Generated-by: following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions) - -EOF -)" -``` - -The `--web` flag opens the browser so the user can review and submit. The `--body` flag -pre-fills the PR template with the generative AI disclosure already completed. - -Remind the user to: - -1. Review the PR title — keep it short (under 70 chars) and focused on user impact. -2. Add a brief description of the changes at the top of the body. -3. Reference related issues when applicable (`closes: #ISSUE` or `related: #ISSUE`). - -### Tracking issues for deferred work - -When a PR applies a **workaround, version cap, mitigation, or partial fix** -rather than solving the underlying problem (for example: upper-binding a -dependency to avoid a breaking upstream release, disabling a feature -behind a flag, reverting a change that needs a better replacement, or -papering over a bug so a release can ship), the deferred work must be -captured in a GitHub tracking issue **and** the tracking issue URL must -appear as a comment at the workaround site in the code. - -1. **Open the tracking issue first**, before finalising the PR body. -2. **Reference it in the PR body by number** — e.g. "full migration is - tracked in #65609" — so anyone reviewing the PR can see what was - deferred and why. -3. **Add a link to the tracking issue as a comment at the workaround - itself**, so the reference survives after the PR merges and anyone - reading the source later can click straight through to the follow-up - work. Use the **full issue URL**, not bare `#NNNNN` — bare references - do not auto-link outside GitHub's web UI (e.g. when grepping in an - editor, browsing a checkout, or reading the file in a terminal). - For example: - - ```toml - # pyproject.toml - # Remove the <1.0 cap after migrating to httpx 1.x; - # tracked at https://github.com/apache/airflow/issues/65609 - "httpx>=0.27.0,<1.0", - ``` - - ```python - # some_module.py - # Delete this fallback once the new client is on all workers; - # tracked at https://github.com/apache/airflow/issues/65609 - if old_client: - ... - ``` - -4. **Do not** write vague forward-looking phrases like "will open a - tracking issue" or "to be filed later" in the PR body or in code - comments. Open the issue, link it in both places, then submit the PR. -5. The tracking issue should describe: what the workaround is, why it - was chosen, the concrete follow-up work needed, and any acceptance - criteria for removing the workaround. - -If a PR you already opened has such forward-looking language, open the -tracking issue, add a PR comment referencing the issue URL, and push a -follow-up commit that adds the tracking-issue URL as a comment at the -workaround site in the code. - -### GitHub messages drafted by agents - -Anything an agent drafts that ends up posted to GitHub on the user's -account — PR / issue comments, PR-level reviews, line-level review -comments, discussion replies — must end with an attribution footer. -The footer is required whether or not a human reviewed the draft -first; what changes between the two cases is the wording. +## Fast test and check commands -Place the footer on its own paragraph at the end of the message, -separated from the body by a blank line and a horizontal rule. Use -the same agent name string used in `Generated-by:` on PR bodies (for -example, `Claude Code (Opus 4.7)`). +- Run scripts tests: `uv run --project scripts pytest scripts/tests/ -xvs` +- Run static checks: `prek run --from-ref --stage pre-commit` +- Run manual checks: `prek run --from-ref --stage manual` +- Run one test: `uv run --project pytest path/to/test.py::TestClass::test_method -xvs` -- **Agent draft, posted without prior human review** (autonomous / - routine work, scheduled triage, etc.): +## Quality gates - ``` - --- - Drafted-by: (no human review before posting) - ``` +Before marking a task complete: -- **Agent draft, reviewed and approved by a human maintainer before - posting:** +1. Run static checks for changed files: + - `prek run --from-ref --stage pre-commit` +2. Run manual checks when required: + - `prek run --from-ref --stage manual` +3. Run relevant tests and confirm they pass. +4. Validate branch diff contains only intentional task files. - ``` - --- - Drafted-by: ; reviewed by @ before posting - ``` +## Skill loading - The `@` is the human who actually read the draft - and said "post it as-is" (or similar). It is not the user the agent - is "running on behalf of" if no review took place — that case is the - first form, not this one. +Machine-readable skills are under `.agents/skills/`. Skills are NOT loaded +automatically — load them on demand when the task requires it. -This footer is in addition to, not a replacement for, any per-tool -disclosure rules (the PR body still keeps its own `Generated-by:` -block under the AI-disclosure checkbox; commit messages still follow -the no-self-as-co-author rule above). Do not skip the footer to -shorten a message — attribution applies regardless of message length. +- When staging files for commit → load `.agents/skills/stage-changes/SKILL.md` +- When running static checks → load `.agents/skills/run-static-checks/SKILL.md` +- When running tests → load `.agents/skills/run-unit-tests/SKILL.md` -## Boundaries +## Required deeper rules -- **Ask first** - - Large cross-package refactors. - - New dependencies with broad impact. - - Destructive data or migration changes. -- **Never** - - Commit secrets, credentials, or tokens. - - Edit generated files by hand when a generation workflow exists. - - Use destructive git operations unless explicitly requested. +Load `CONSTITUTION.md` before making code changes or preparing a PR. -## References +## Source-of-truth docs -- [`contributing-docs/03a_contributors_quick_start_beginners.rst`](contributing-docs/03a_contributors_quick_start_beginners.rst) -- [`contributing-docs/05_pull_requests.rst`](contributing-docs/05_pull_requests.rst) -- [`contributing-docs/07_local_virtualenv.rst`](contributing-docs/07_local_virtualenv.rst) -- [`contributing-docs/08_static_code_checks.rst`](contributing-docs/08_static_code_checks.rst) -- [`contributing-docs/12_provider_distributions.rst`](contributing-docs/12_provider_distributions.rst) -- [`contributing-docs/19_execution_api_versioning.rst`](contributing-docs/19_execution_api_versioning.rst) +- `contributing-docs/03a_contributors_quick_start_beginners.rst` +- `contributing-docs/05_pull_requests.rst` +- `contributing-docs/08_static_code_checks.rst` +- `contributing-docs/09_testing.rst` +- `contributing-docs/19_execution_api_versioning.rst` diff --git a/CONSTITUTION.md b/CONSTITUTION.md new file mode 100644 index 0000000000000..dbe4bbbe72018 --- /dev/null +++ b/CONSTITUTION.md @@ -0,0 +1,167 @@ + + + + +# Project Rules for Code Modifications + +Load this before making any code change, commit, or PR in the Airflow repository. + +## Repository layout + +Airflow is a **uv workspace monorepo**. Key projects: + +| Project | Path | pyproject.toml | +|---|---|---| +| Airflow Core | `airflow-core/` | `airflow-core/pyproject.toml` | +| Task SDK | `task-sdk/` | `task-sdk/pyproject.toml` | +| Airflow CTL | `airflow-ctl/` | `airflow-ctl/pyproject.toml` | +| Breeze (dev tool) | `dev/breeze/` | `dev/breeze/pyproject.toml` | +| Scripts | `scripts/` | `scripts/pyproject.toml` | +| Providers | `providers//` | `providers//pyproject.toml` | +| Helm Chart | `chart/` | — | + +Always pass the correct `--project` when running `uv run` or `pytest`. + +## Shared libraries + +- shared libraries provide implementation of some common utilities like logging, configuration where the code should be reused in different distributions (potentially in different versions) +- we have a number of shared libraries that are separate, small Python distributions located under `shared` folder +- each of the libraries has it's own src, tests, pyproject.toml and dependencies +- sources of those libraries are symbolically linked to the distributions that are using them (`airflow-core`, `task-sdk` for example) +- tests for the libraries (internal) are in the shared distribution's test and can be run from the shared distributions +- tests of the consumers using the shared libraries are present in the distributions that use the libraries and can be run from there + +## Host vs Breeze boundary + +This is the most common source of agent errors. + +| Operation | Where to run | Why | +|---|---|---| +| `git add`, `git commit`, `git push` | **Host only** | `.git` is not mounted inside Breeze | +| `pytest` (unit tests) | Host (`uv run`) or Breeze (`breeze run`) | System deps may require Breeze | +| `prek` (static checks) | Either | Works identically in both | +| `breeze shell`, `breeze run` | **Host only** (launches Breeze) | These are host CLI commands | + +**Detection:** `BREEZE` environment variable is set → you are inside Breeze. Otherwise → host. + +## DevContainers and Worktrees + +- **DevContainers:** When running inside a devcontainer, the environment is typically pre-configured with all system dependencies and the `.git` directory is mounted. In this context, the `BREEZE` environment variable is **not** set. Agents should treat DevContainers as a "Host" environment where `git` and `uv` commands work directly without needing Breeze. +- **Git Worktrees:** Agents fully support git worktrees. When working in a worktree, the `scripts/tools/setup_breeze` command installs a shim that correctly routes `breeze` commands to the local source tree, ensuring that Breeze always runs the code from the current worktree. + +## Commands + +`` is the folder containing the relevant `pyproject.toml`, e.g. `airflow-core`, `providers/amazon`, `task-sdk`, `scripts`. +`` is the branch the PR targets — usually `main`. + +- **Run a single test:** `uv run --project pytest path/to/test.py::TestClass::test_method -xvs` +- **Run a test file:** `uv run --project pytest path/to/test.py -xvs` +- **Run all tests in package:** `uv run --project pytest path/to/package -xvs` +- **If uv fails due to missing system deps, fall back to Breeze:** `breeze run pytest -xvs` +- **Run scripts tests:** `uv run --project scripts pytest scripts/tests/ -xvs` +- **Run core or provider tests in parallel:** `breeze testing --run-in-parallel` (groups: `core-tests`, `providers-tests`) +- **Run Airflow CLI:** `breeze run airflow dags list` +- **Type-check (non-providers):** `prek run mypy- --all-files` +- **Type-check (providers):** `breeze run mypy path/to/code` +- **Static checks:** `prek run --from-ref --stage pre-commit` +- **Manual checks:** `prek run --from-ref --stage manual` +- **Build docs:** `breeze build-docs` +- **Find impacted tests:** `breeze selective-checks --commit-ref ` + +SQLite is the default backend. Use `--backend postgres` or `--backend mysql` for integration tests that need those databases. + +## Security Model + +The authoritative reference is `airflow-core/docs/security/security_model.rst`. + +**Distinguish between:** + +1. **Actual vulnerabilities** — code violating the documented model (e.g., worker gaining DB access). +2. **Known limitations** — documented gaps tracked for future improvement (e.g., DFP/Triggerer DB access). +3. **Deployment hardening** — measures for Deployment Managers, not code-level issues. + +## Coding standards + +- **Always format and check Python files with ruff immediately after writing or editing them:** `uv run ruff format ` and `uv run ruff check --fix `. Do this for every Python file you create or modify, before moving on to the next step. +- **No `assert` in production**: Use standard exceptions instead. +- **Exceptions**: Define dedicated exception classes or use existing exceptions such as `ValueError` instead of raising the broad `AirflowException` directly. +- **Session handling**: In `airflow-core`, functions with a `session` parameter must not call `session.commit()`. Use keyword-only `session` parameters. +- **Imports**: Imports at top of file. Valid exceptions: circular imports, lazy loading for worker isolation, `TYPE_CHECKING` blocks. +- **Duration**: Use `time.monotonic()`, not `time.time()`. +- **Operators**: Only assign fields in `__init__`. Validate in `execute()`. + +## Testing Standards + +- Add tests for new behavior — cover success, failure, and edge cases. +- Use pytest patterns, not `unittest.TestCase`. +- Use `spec`/`autospec` when mocking. +- Use `time_machine` for time-dependent tests. Do not use `datetime.now()` +- Use `@pytest.mark.parametrize` for multiple similar inputs. +- Use `@pytest.mark.db_test` for tests that require database access. +- Test fixtures: `devel-common/src/tests_common/pytest_plugin.py`. +- Test location mirrors source: `airflow/cli/cli_parser.py` → `tests/cli/test_cli_parser.py`. +- Do not use `caplog` in tests, prefer checking logic and not log output. + +## Commits and PRs + +Write commit messages focused on user impact, not implementation details. + +- **Good:** `Fix airflow dags test command failure without serialized Dags` +- **Good:** `UI: Fix Grid view not refreshing after task actions` +- **Bad:** `Initialize DAG bundles in CLI get_dag function` + +Add a newsfragment for user-visible changes: +`echo "Brief description" > airflow-core/newsfragments/{PR_NUMBER}.{bugfix|feature|improvement|doc|misc|significant}.rst` + +- **AI Disclosure:** If generative AI was used, include a disclosure in the PR body using the project template. +- **Attribution:** Any agent-drafted messages posted to GitHub (comments, reviews) must end with an attribution footer: + + ``` + --- + Drafted-by: (no human review before posting) + ``` + +- **Co-Authored-By:** NEVER add `Co-Authored-By` with yourself as co-author. Agents are assistants, not authors. + +### Git remote naming conventions + +Airflow standardises on two git remote names: **`upstream`** (canonical repo) and **`origin`** (your fork). Always push branches to `origin`. + +### Tracking issues for deferred work + +- When applying a workaround, version cap, mitigation, or partial fix rather than solving the underlying problem (e.g., upper-binding a dependency to avoid a breaking upstream release), the deferred work must be captured in a GitHub tracking issue. +- **Link in Code:** The tracking issue URL must appear as a comment at the workaround site in the code. Use the full issue URL (e.g., `tracked at https://github.com/apache/airflow/issues/65609`). +- **PR Description:** Reference the issue in the PR body (e.g., "full migration is tracked in #65609"). + + +## Boundaries + +- **Ask First**: Large refactors, new dependencies, destructive migrations. +- **Never**: Commit secrets, edit generated files manually, or use destructive git operations unless asked. + +## References + +- `contributing-docs/03a_contributors_quick_start_beginners.rst` +- `contributing-docs/05_pull_requests.rst` +- `contributing-docs/07_local_virtualenv.rst` +- `contributing-docs/08_static_code_checks.rst` +- `contributing-docs/12_provider_distributions.rst` +- `contributing-docs/19_execution_api_versioning.rst` diff --git a/scripts/ci/prek/breeze_context.py b/scripts/ci/prek/breeze_context.py new file mode 100644 index 0000000000000..a1e8e9f469df2 --- /dev/null +++ b/scripts/ci/prek/breeze_context.py @@ -0,0 +1,252 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Breeze context detection and command selection. + +This module exists for agent workflows that must reliably choose between: + +- "host" execution (commands intended to run on the contributor machine) +- "breeze" execution (commands intended to run inside the Breeze container) + +Architecture +------------ +Agents call ``BreezeContext.get_command`` with a skill id and parameters. +The helper detects the current runtime context with ``BreezeContext.is_in_breeze``, +optionally using a manual override for debugging/testing. + +Detection strategy +------------------ +Check if the ``BREEZE`` environment variable is set. Breeze sets this variable +when starting the container, so its presence reliably indicates a Breeze environment. +In devcontainers, this variable is typically not set, which is intentional: devcontainers +behave like a host environment where tools like `git` and `uv` are directly available. + +Testing +------- +This module is covered by unit tests: +- `scripts/tests/test_breeze_context.py` +- `scripts/tests/test_edge_cases.py` + +For manual debugging you can run: +`python3 scripts/ci/prek/breeze_context.py --force-context host` +or +`python3 scripts/ci/prek/breeze_context.py --force-context breeze` +""" + +from __future__ import annotations + +import argparse +import logging +import os +import shlex +from typing import Any, TypedDict + +logger = logging.getLogger(__name__) + + +class SkillParam(TypedDict): + required: bool + + +class SkillDef(TypedDict): + host: str + breeze: str | None + preferred: str + params: dict[str, SkillParam] + + +class BreezeContext: + """Detect whether code is running in Breeze container or on host.""" + + @staticmethod + def is_in_breeze(force_context: str | None = None) -> bool: + """ + Detect whether the current process is running inside Breeze. + + Parameters + ---------- + force_context: + Optional manual override. + - ``"breeze"`` forces Breeze context. + - ``"host"`` forces host context. + - ``None`` uses auto-detection. + + Returns + ------- + bool + ``True`` when running in Breeze, otherwise ``False``. + + Raises + ------ + ValueError + If ``force_context`` is not one of ``"host"`` or ``"breeze"``. + """ + if force_context is not None: + normalized = force_context.strip().lower() + if normalized == "breeze": + logger.info("Context forced to breeze via --force-context") + return True + if normalized == "host": + logger.info("Context forced to host via --force-context") + return False + raise ValueError(f"Invalid force_context={force_context!r}. Expected 'host' or 'breeze'.") + + # Check BREEZE env variable — Breeze sets this when starting the container. + if os.environ.get("BREEZE"): + logger.debug("Detected Breeze via BREEZE environment variable") + return True + + logger.debug("BREEZE environment variable not set; assuming host execution") + return False + + @staticmethod + def get_command(skill_id: str, *, force_context: str | None = None, **kwargs) -> str: + """ + Generate the appropriate command for the given skill and context. + + Args: + skill_id: Unique skill identifier (e.g., "run-unit-tests") + **kwargs: Skill-specific parameters + + Returns: + Command string to execute + + Raises: + ValueError: If ``skill_id`` is unknown or required parameters are missing. + """ + is_breeze = BreezeContext.is_in_breeze(force_context=force_context) + context = "breeze" if is_breeze else "host" + + # Define all available skills + skills: dict[str, Any] = { + "stage-changes": { + # git add operates on the host working tree — not inside the container. + # This is explicitly a host-only operation per the contributor workflow. + "host": "git add {path}", + "breeze": None, + "preferred": "host", + "params": {"path": {"required": True}}, + }, + "run-static-checks": { + # prek is available on both host and inside Breeze; same command in both contexts. + # --from-ref is required; --stage defaults to pre-commit if omitted. + "host": "prek run --from-ref {from_ref} --stage pre-commit", + "breeze": "prek run --from-ref {from_ref} --stage pre-commit", + "preferred": "host", + "params": {"from_ref": {"required": True}}, + }, + "run-unit-tests": { + # project is the folder containing pyproject.toml, e.g. airflow-core, providers/amazon. + "host": "uv run --project {project} pytest {test_path}", + "breeze": "breeze run pytest {test_path}", + "preferred": "host", + "params": {"project": {"required": True}, "test_path": {"required": True}}, + }, + } + + # Validate skill exists + if skill_id not in skills: + available = ", ".join(sorted(skills.keys())) + raise ValueError(f"Skill not found: {skill_id}. Available skills: {available}") + + skill: SkillDef = skills[skill_id] + + # Validate required parameters + params = skill["params"] + for param_name, param_info in params.items(): + if param_info["required"] and param_name not in kwargs: + raise ValueError(f"Missing required parameter: {param_name} for skill {skill_id!r}") + + # Choose context; guard host-only skills + if context == "breeze": + command_template = skill["breeze"] + else: + command_template = skill["host"] + if command_template is None: + raise ValueError( + f"Skill {skill_id!r} is host-only and cannot be run inside Breeze. " + f"Run this command from your host terminal before switching to Breeze." + ) + + # Substitute parameters + command: str = command_template + for param_name, _ in params.items(): + placeholder = "{" + param_name + "}" + if param_name in kwargs: + command = command.replace(placeholder, shlex.quote(str(kwargs[param_name]))) + else: + command = command.replace(placeholder, "") + + # Clean up extra whitespace that may remain after optional param removal + command = " ".join(command.split()) + + logger.debug( + "Generated command for skill_id=%r context=%s kwargs=%r: %s", + skill_id, + context, + kwargs, + command, + ) + return command + + +def detect_context(force_context: str | None = None) -> str: + """Return ``host`` or ``breeze`` based on current environment. + + Parameters + ---------- + force_context: + Optional override (``host`` or ``breeze``). + + Returns + ------- + str + ``"breeze"`` or ``"host"``. + """ + return "breeze" if BreezeContext.is_in_breeze(force_context=force_context) else "host" + + +def _build_arg_parser() -> argparse.ArgumentParser: + """Build the CLI argument parser for this module.""" + parser = argparse.ArgumentParser(description="Detect Breeze context for agent skills") + parser.add_argument( + "--force-context", + choices=["host", "breeze"], + default=None, + help="Override auto-detection and force host or breeze context", + ) + parser.add_argument( + "--verbose", + action="store_true", + help="Enable debug logging for context detection steps", + ) + return parser + + +def main(argv: list[str] | None = None) -> int: + """Entry point for manual debugging.""" + parser = _build_arg_parser() + args = parser.parse_args(argv) + + logging.basicConfig(level=logging.DEBUG if args.verbose else logging.INFO) + print(detect_context(force_context=args.force_context)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/ci/prek/validate_skills.py b/scripts/ci/prek/validate_skills.py new file mode 100755 index 0000000000000..1e5d59f4cff2b --- /dev/null +++ b/scripts/ci/prek/validate_skills.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Validate Agent Skills adherence to the agentskills.io standard. + +This script is used by the project's prek/pre-commit pipeline to ensure that: + +- All skills in `.agents/skills` follow the valid directory structure +- SKILL.md files have proper YAML frontmatter with `name` and `description` +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +try: + import yaml +except ImportError: + print("PyYAML is required for validate_skills.py. Install with `pip install pyyaml`.") + sys.exit(1) + +SKILLS_DIR = Path(".agents/skills") + + +def validate_skill(skill_dir: Path) -> list[str]: + """Validate a single skill directory and return a list of errors.""" + errors = [] + + skill_md = skill_dir / "SKILL.md" + if not skill_md.exists(): + errors.append(f"Missing SKILL.md in {skill_dir}") + return errors + + content = skill_md.read_text(encoding="utf-8") + + # Strip leading HTML comment blocks (e.g. SPDX license and markdownlint + # pragmas inserted/maintained by hooks) before checking for YAML frontmatter. + stripped = content.lstrip() + while stripped.startswith("") + if end_comment == -1: + break + stripped = stripped[end_comment + 3 :].lstrip() + + # Normalize newlines to \n to support CRLF (Windows) checkouts + stripped = stripped.replace("\r\n", "\n") + + if not stripped.startswith("---\n"): + errors.append(f"Missing YAML frontmatter at start of {skill_md}") + return errors + + parts = stripped.split("---\n") + if len(parts) < 3: + errors.append(f"Incomplete YAML frontmatter in {skill_md}") + return errors + + frontmatter_str = parts[1] + try: + frontmatter = yaml.safe_load(frontmatter_str) + except yaml.YAMLError as e: + errors.append(f"Invalid YAML in {skill_md}: {e}") + return errors + + if not isinstance(frontmatter, dict): + errors.append(f"YAML frontmatter in {skill_md} must be a dictionary") + return errors + + if "name" not in frontmatter: + errors.append(f"Missing 'name' in {skill_md} frontmatter") + elif frontmatter["name"] != skill_dir.name: + errors.append(f"Skill name '{frontmatter['name']}' does not match directory name '{skill_dir.name}'") + + if "description" not in frontmatter: + errors.append(f"Missing 'description' in {skill_md} frontmatter") + + return errors + + +def main(argv: list[str] | None = None) -> int: + if not SKILLS_DIR.exists(): + print(f"Directory {SKILLS_DIR} not found.") + return 0 + + all_errors = [] + for skill_dir in SKILLS_DIR.iterdir(): + if skill_dir.is_dir(): + errors = validate_skill(skill_dir) + all_errors.extend(errors) + + if all_errors: + print("❌ Agent Skills Validation Failed:") + for error in all_errors: + print(f" - {error}") + return 1 + + print("✓ Agent Skills conform to standard") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/test_breeze_context.py b/scripts/tests/test_breeze_context.py new file mode 100644 index 0000000000000..a30a28eb94b72 --- /dev/null +++ b/scripts/tests/test_breeze_context.py @@ -0,0 +1,95 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for breeze_context.py - Context detection module.""" + +from __future__ import annotations + +import os +from unittest.mock import patch + +import pytest +from ci.prek.breeze_context import BreezeContext + + +class TestBreezeContextDetection: + """Test context detection logic.""" + + def test_breeze_detected_by_env_var(self): + """Should detect Breeze when BREEZE env var is set.""" + with patch.dict(os.environ, {"BREEZE": "true"}): + assert BreezeContext.is_in_breeze() is True + + def test_host_detected_by_default(self): + """Should detect host environment when BREEZE env var is not set.""" + env = {k: v for k, v in os.environ.items() if k != "BREEZE"} + with patch.dict(os.environ, env, clear=True): + assert BreezeContext.is_in_breeze() is False + + def test_force_context_breeze(self): + """Should return True when force_context is 'breeze'.""" + assert BreezeContext.is_in_breeze(force_context="breeze") is True + + def test_force_context_host(self): + """Should return False when force_context is 'host'.""" + assert BreezeContext.is_in_breeze(force_context="host") is False + + def test_invalid_force_context_raises(self): + """Should raise ValueError for invalid force_context.""" + with pytest.raises(ValueError, match="Invalid force_context"): + BreezeContext.is_in_breeze(force_context="invalid") + + +class TestGetCommand: + """Test command generation for different contexts.""" + + def test_get_static_checks_command_on_host(self): + """Should return prek command when on host.""" + env = {k: v for k, v in os.environ.items() if k != "BREEZE"} + with patch.dict(os.environ, env, clear=True): + cmd = BreezeContext.get_command("run-static-checks", from_ref="main") + assert "prek" in cmd + assert "--from-ref" in cmd + assert "main" in cmd + assert "--stage pre-commit" in cmd + + def test_get_unit_tests_command_on_host(self): + """Should return uv pytest command when on host.""" + env = {k: v for k, v in os.environ.items() if k != "BREEZE"} + with patch.dict(os.environ, env, clear=True): + cmd = BreezeContext.get_command("run-unit-tests", project="airflow-core", test_path="tests/api/") + assert "uv run" in cmd + assert "pytest" in cmd + + def test_get_unit_tests_command_in_breeze(self): + """Should return breeze run command when in Breeze.""" + with patch.dict(os.environ, {"BREEZE": "true"}): + cmd = BreezeContext.get_command("run-unit-tests", project="airflow-core", test_path="tests/api/") + assert "breeze run" in cmd + assert "pytest" in cmd + + def test_missing_required_parameter_raises_error(self): + """Should raise ValueError when required parameter missing.""" + env = {k: v for k, v in os.environ.items() if k != "BREEZE"} + with patch.dict(os.environ, env, clear=True): + with pytest.raises(ValueError, match="test_path"): + BreezeContext.get_command("run-unit-tests", project="airflow-core") + + def test_skill_not_found_raises_error(self): + """Should raise ValueError when skill doesn't exist.""" + with pytest.raises(ValueError, match="Skill not found"): + BreezeContext.get_command("nonexistent-skill") diff --git a/scripts/tests/test_contributor_scenario.py b/scripts/tests/test_contributor_scenario.py new file mode 100644 index 0000000000000..ba8dfd7dc38cf --- /dev/null +++ b/scripts/tests/test_contributor_scenario.py @@ -0,0 +1,75 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Test simulating a full human/agent contributor workflow (the 'Exam').""" + +from __future__ import annotations + +import os +from unittest.mock import patch + +import pytest +from ci.prek.breeze_context import BreezeContext + + +class TestContributorScenario: + """Verifies the standard workflow (git add -> prek -> pytest) models correctly.""" + + def test_end_to_end_host_contribution_workflow(self): + """Simulates an agent running the standard workflow entirely on the host.""" + with patch.dict(os.environ, {}, clear=True): + with patch("os.path.exists", return_value=False): + with patch("os.path.isdir", return_value=False): + assert BreezeContext.is_in_breeze() is False + + # Step 1: Stage changes + cmd1 = BreezeContext.get_command("stage-changes", path="airflow/models/dag.py") + assert cmd1 == "git add airflow/models/dag.py" + + # Step 2: Run static checks + cmd2 = BreezeContext.get_command("run-static-checks", from_ref="main") + assert cmd2 == "prek run --from-ref main --stage pre-commit" + + # Step 3: Run targeted unit tests + cmd3 = BreezeContext.get_command( + "run-unit-tests", + project="airflow-core", + test_path="airflow-core/tests/models/test_dag.py", + ) + assert ( + cmd3 == "uv run --project airflow-core pytest airflow-core/tests/models/test_dag.py" + ) + + def test_host_only_guard_in_breeze(self): + """Simulates an agent attempting to run a host-only command while inside Breeze context.""" + with patch.dict(os.environ, {"BREEZE": "true"}): + assert BreezeContext.is_in_breeze() is True + + # Step 1: Attempt to stage changes (should fail because git is host-only) + with pytest.raises(ValueError, match="host-only"): + BreezeContext.get_command("stage-changes", path="airflow/models/dag.py") + + # Step 2 & 3: Still work + cmd2 = BreezeContext.get_command("run-static-checks", from_ref="main") + assert cmd2 == "prek run --from-ref main --stage pre-commit" + + cmd3 = BreezeContext.get_command( + "run-unit-tests", + project="airflow-core", + test_path="airflow-core/tests/models/test_dag.py", + ) + assert cmd3 == "breeze run pytest airflow-core/tests/models/test_dag.py" diff --git a/scripts/tests/test_edge_cases.py b/scripts/tests/test_edge_cases.py new file mode 100644 index 0000000000000..6e89c75de71e2 --- /dev/null +++ b/scripts/tests/test_edge_cases.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python3 +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Additional edge-case tests for Breeze context.""" + +from __future__ import annotations + +import os +from unittest.mock import patch + +from ci.prek.breeze_context import BreezeContext + + +class TestEdgeCases: + """Edge cases around context detection.""" + + def test_context_detects_host_without_breeze_var(self) -> None: + """Should default to host when BREEZE env var is not set.""" + env = {k: v for k, v in os.environ.items() if k != "BREEZE"} + with patch.dict(os.environ, env, clear=True): + assert BreezeContext.is_in_breeze() is False + + def test_context_detects_breeze_with_breeze_var(self) -> None: + """Should detect Breeze when BREEZE env var is set.""" + with patch.dict(os.environ, {"BREEZE": "true"}): + assert BreezeContext.is_in_breeze() is True + + def test_parameter_substitution_handles_quotes_spaces(self) -> None: + """Command template substitution should preserve special characters via escaping.""" + import shlex + + test_path = "tests/unit/my file's test.py" + cmd = BreezeContext.get_command("run-unit-tests", project="airflow-core", test_path=test_path) + assert shlex.quote(test_path) in cmd From 964970ece8a6bbc25e2855d058939b3584e76951 Mon Sep 17 00:00:00 2001 From: Subham Sangwan Date: Tue, 26 May 2026 08:44:03 +0530 Subject: [PATCH 2/2] Fix static checks: add trailing newline to AGENTS.md --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 4cbac913b5616..a713cc3bdf90f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,4 +54,4 @@ Load CONSTITUTION.md before making code changes or preparing a PR. - `contributing-docs/05_pull_requests.rst` - `contributing-docs/08_static_code_checks.rst` - `contributing-docs/09_testing.rst` -- `contributing-docs/19_execution_api_versioning.rst` \ No newline at end of file +- `contributing-docs/19_execution_api_versioning.rst`