# ADR-0002: Ansible Check Tool Consolidation and wait_for_checks Extraction

Date: 2026-08-12
Status: Accepted

## Context

The devx package had two categories of code duplication and inline
workflow logic that were hard to test and maintain:

### 1. Ansible Check Tools — Duplicated Boilerplate

Five Ansible check tools (`check_ansible_no_log`,
`check_ansible_patterns`, `check_ansible_set_fact_to_json`,
`check_ansible_no_state_absent_on_db`, `check_jinja_expr`) each
implemented their own file discovery, YAML parsing, task iteration, and
violation reporting logic. While the check logic differed, the
supporting infrastructure was copy-pasted across all five modules:

- `find_task_files()` — glob YAML files, skip molecule
- YAML multi-document parsing with error handling
- Task iteration (bare lists, play dicts with `tasks`/`pre_tasks`/`post_tasks`/`handlers`, nested `block` tasks)
- Violation formatting (`path:line — message`)

This made it difficult to add new checks (each new tool repeated the
boilerplate) and risky to change shared behavior (fixes had to be
applied to all five modules independently).

### 2. Inline Job Polling in Workflow YAML

The `grm` repository's `ci.yml` workflow contained ~25 lines of inline
shell + Python polling logic to wait for the `molecule-tests` job to
complete before the auto-merge step. This logic:

- Was not testable (embedded in workflow YAML)
- Duplicated the Gitea API client pattern already used elsewhere
- Had no timeout handling, no error reporting, no retry logic
- Could not be reused by other repositories

### 3. Duplicate discover_runners Modules

`devx.ci.discover_runners` and `devx.molecule.discover_runners` were
near-identical modules. The `ci/` version had better error logging
(warnings on non-200 responses, 403 suppression for instance-level
queries), while the `molecule/` version silently swallowed errors.
Both were imported by different workflows, making it unclear which was
canonical.

## Decision

### 1. Composable `ansible_checks/` Subpackage

Consolidate the five Ansible check tools into a
`devx.tools.ansible_checks/` subpackage with shared utilities:

- `_shared.py` — `AnsibleFileFinder`, `AnsibleYAMLParser`,
  `ViolationReporter` classes providing composable helpers
- `no_log.py`, `patterns.py`, `set_fact_to_json.py`,
  `no_state_absent_on_db.py`, `jinja_expr.py` — canonical check
  implementations using the shared utilities

The old modules (`check_ansible_*.py`, `check_jinja_expr.py`) remain as
**thin backward-compat wrappers** that re-export the canonical
implementation and preserve the CLI entry point. This avoids breaking
existing Makefile targets and workflow references.

**Composition over inheritance**: each check module picks the helpers it
needs. Tools that don't parse YAML (for example line-based scanners) can skip
`AnsibleYAMLParser` entirely.

### 2. Extracted `wait_for_checks` Module

Extract the inline polling logic into `devx.ci.wait_for_checks`:

- Polls the Gitea API for job completion status
- Configurable job name prefix, timeout, poll interval
- Exit codes: 0 (success), 1 (failure), 2 (timeout), 3 (API error)
- `--require-success/--no-require-success` flag for flexibility
- 100% test coverage with mocked API responses

This replaces the inline shell polling in `grm` `ci.yml` with a
reusable, testable Python module.

### 3. Deprecated `ci/discover_runners` Wrapper

Merge the `ci/discover_runners` implementation (with its better error
logging) into `molecule/discover_runners` as the canonical version.
Make `ci/discover_runners` a deprecated wrapper that:

- Re-exports all public symbols from `molecule.discover_runners`
- Emits a `DeprecationWarning` when run as `__main__`
- Preserves backward compatibility for existing workflow references

New code should import from `devx.molecule.discover_runners` directly.

## Consequences

- **New checks are easier to write**: import `_shared` helpers, implement
  only the check-specific logic
- **Shared behavior can be fixed in one place**: file discovery, YAML
  parsing, violation formatting
- **Workflow polling is testable**: `wait_for_checks` has 26 unit tests
  covering success, failure, timeout, and API error scenarios
- **Backward compatibility preserved**: all existing Makefile targets,
  workflow references, and test imports continue to work via wrappers
- **Migration path is gradual**: new code uses the subpackage; old code
  can migrate at its own pace; wrappers can be removed in a future
  release once all references are updated