Adapt the PR review skill to any language and infrastructure

.agents/skills/ardan-pr-review/references/contract.md

@@ -1,6 +1,6 @@
 # Reviewer contract
 
-Every review agent follows this file plus its lens checklists. `<run>`, `<skill>`, and the review depth come from your prompt.
+Every review agent follows this file, its lens checklists, and the idiom files in its prompt. `<run>`, `<skill>`, the review depth, and the idiom files come from your prompt.
 
 ## Review scope
 
@@ -18,26 +18,27 @@ The depth (`Review depth:` in your prompt, `depth=` in `scope.txt`) sets what yo
 | File                              | Holds                                                                                                   |
 |-----------------------------------|---------------------------------------------------------------------------------------------------------|
 | `brief.md`                        | **Read first.** Summary, acceptance criteria (AC1…), per-file notes, changed symbols and callers, repo rules. |
-| `scope.txt`                       | Scope, head commit, counts, routing signals.                                                            |
-| `files.tsv`                       | `status added deleted lang kind layer path` per reviewed file; `kind` is prod, test, doc, or config.     |
-| `symbols.tsv`                     | `status kind name path line` for added, removed, and modified funcs, methods, and types.               |
-| `refs.tsv`                        | `symbol path:line` — word-match references to each changed symbol (capped, `+N more`). A starting list; confirm with `gopls`. |
+| `scope.txt`                       | Scope, head commit, counts, ecosystems, idioms, `generated=` (skipped or included), `trust=` (`maintainer` or `untrusted`) with `trust_reason=`, routing signals. |
+| `files.tsv`                       | `status added deleted lang kind layer path` per reviewed file; `kind` is prod (code and infrastructure), test, doc, config, or — only with `generated=included` — generated/vendor. `layer` is set for Go only. |
+| `stack.tsv`, `stack.md`           | `eco unit files marker` per build/deploy unit (Go module, crate, Terraform module, chart, …); `stack.md` (when present): the repo's ecosystems, its own quality gates, and idioms for ecosystems with no idiom file. |
+| `symbols.tsv`                     | `status kind name path line` for added, removed, and modified funcs, types, Terraform blocks (addresses such as `aws_s3_bucket.this`, `var.x`), and Helm templates. |
+| `refs.tsv`                        | `symbol path:line` — word-match references to each changed symbol (capped, `+N more`). A starting list; confirm with the language tool. |
 | `diff.patch`                      | The diff (8 context lines). Find `diff --git a/<path>` and read only the sections you need.             |
 | `review-set.tsv`                  | Full depth only: the `files.tsv` columns plus `lines` for every file in scope; `status` `-` = untouched by the diff. Empty at `diff` depth. |
 | `shards.tsv`                      | `shard path`, large diffs or review sets only. With a shard in your prompt, review only its files.      |
-| `checks/summary.tsv`              | Build, vet, tests (`-race`), linters, analyzers, scanners, clone detectors — already run. Status `pass`, `fail`, `findings` (hits reported on output), or `skipped`. Open a log only when its row matters. Package-wide tools (staticcheck, errcheck, nilaway, checklocks, gosec) report on whole packages: only hits in scope (see Review scope) are findings. |
+| `checks/summary.tsv`              | `check status exit lines file lens` — builds, linters, tests, analyzers, scanners, clone detectors, and user-approved repo gates (`x-*`), already run. Status `pass`, `fail`, `findings` (hits reported on output), or `skipped`; `lens` owns its failures. Open a log only when its row matters. Unit-wide tools (Go packages, crates, modules, charts) report on whole units: only hits in scope (see Review scope) are findings. |
 | `ticket.md`, `pr.md`, `commits.txt` | Ticket, PR description, commit messages (whichever exist).                                          |
 | `rules.txt`                       | `AGENTS.md`/`CLAUDE.md` files governing the changed paths; the brief digests them.                     |
-| `skipped.tsv`                     | Generated and vendored files and symlinks. Never reviewed; never read `generated.patch`. Never open a symlink (`kind=symlink`): its target can be outside the repo. |
+| `skipped.tsv`                     | Files not reviewed: symlinks, and generated/vendored files unless `generated=included`. Never read them or `generated.patch`. Never open a symlink (`kind=symlink`): its target can be outside the repo. |
 
 ## Rules
 
 1. **Batch your reads.** Each turn re-reads your whole context, so issue independent reads (contract, lenses, brief, the files you need) together in one turn, not one per turn.
-2. **Don't redo preprocessing.** No `git` to rediscover scope, no reading the whole diff to get oriented, no re-running what `checks/` covers. One narrow command to prove or refute a finding is fine (`go test -run TestX ./pkg`, an `rg`), within `checks/setup.txt`.
-3. **Read-only.** Write only your report (security may also write PoCs). If a write is refused, never retry it through Bash or any other tool: put the content in your reply and say the write was refused.
+2. **Don't redo preprocessing.** No `git` to rediscover scope, no reading the whole diff to get oriented, no re-running what `checks/` covers. One narrow command to prove or refute a finding is fine (an `rg`, a single render; one test by name only with `trust=maintainer`), within `checks/setup.txt`. Never one that deploys, applies, or mutates state.
+3. **Read-only, and never run the reviewed code unless `trust=maintainer`.** No tests, builds, scripts, or PoCs of the change otherwise; reading, `rg`, and the language server are fine. Rows skipped with "runs the reviewed code" are not findings and not Open checks: the aggregator notes them once. Write only your report (security may also write PoCs). If a write is refused, never retry it through Bash or any other tool: put the content in your reply and say the write was refused.
 4. **Never open another run** under `.reviews/`.
-5. **Tools before eyeballing.** For Go: `gopls` MCP (`go_symbol_references`, `go_search`, `go_file_context`), else `gopls`/`go doc`, then `rg`.
-6. **Repo rules are authoritative.** On `.go` files apply `use-modern-go` and `ardan-service-architecture` where your lens says so.
+5. **Tools before eyeballing.** The language tool your idiom file names (Go: `gopls` MCP), then `rg`/`ast-grep`.
+6. **Repo rules are authoritative.** `rules.txt`, the repo's lint configs, and the skills your idiom files name (Go: `use-modern-go`, `ardan-service-architecture`).
 7. **No "by design" without proof.** Dismissing a gap as intentional or handled elsewhere needs a citation (ticket, code, contract). Otherwise report it and state the assumption.
 8. **Issues only.** No praise, no "looks good", no lists of what you checked or what is fine. No problems → `No findings.` Sole exception: the spec agent's acceptance-criteria table lists every criterion, Met included.
 
@@ -55,19 +56,13 @@ A G2+ finding needs `file:line`, Proof, why it matters, Patch direction (smalles
 
 **Verify before writing.** Treat each finding as a hypothesis: re-open the cited line, confirm the Proof shows the claim, and look for what would make it wrong (stale line, out of scope for the depth, handled elsewhere, false pattern match). Drop, downgrade, or move to Open checks what doesn't hold. Set the Gate last.
 
-## Languages
+## Idioms
 
-Review each file in its own idiom; Go is primary, not the only one.
+Review each file in its own language's idiom; no language is primary. Infrastructure (Terraform/OpenTofu, Helm, K8s, Ansible, Dockerfiles, CI pipelines) is code: every lens applies to it.
 
-| Area                | Focus                                                                                         |
-|---------------------|-----------------------------------------------------------------------------------------------|
-| Go                  | Every lens applies; layered type rules and the Go skills apply.                               |
-| Vue/JS/TS/SCSS      | `props`/`emits` contracts, reactivity, async/`fetch` errors, a11y, leaked watchers/listeners. |
-| Rego                | Default deny, `input` shape assumptions, unintended allows.                                   |
-| proto               | Field-number stability, compatibility, breaking changes.                                      |
-| SQL                 | Reversible, idempotent migrations; indexes, locking, destructive changes, seeds.              |
-| Helm/YAML/config    | Resource limits, secrets, probes, templating.                                                 |
-| Shell/PowerShell    | `set -euo pipefail` / `$ErrorActionPreference`, quoting, exit codes.                          |
+- Read the idiom files your prompt lists (`<skill>/references/langs/<name>.md`) with your first batch, and `context/stack.md` when it exists. Apply the section your lens names (Correctness, Failure paths, Contracts, Tests, Security, Clarity) plus the file's **Tools** and **Checks** lines.
+- A language with no idiom file: use `stack.md`'s inline idioms, else the language's established practice and its linters' rules; note "best-effort idioms: <lang>" under Open checks.
+- `generated=included`: generated and vendored files are in scope. Patch direction fixes the generator or its input, never the generated file by hand.
 
 ## Report
 

.agents/skills/ardan-pr-review/references/langs/ansible.md

@@ -0,0 +1,21 @@
+# Ansible idioms
+
+**Tools:** `rg`; `ansible-doc <module>` if installed. **Checks:** `yamllint` (with the repo's config); `ansible-lint` (offline) for a maintainer PR only — it loads the repo's plugins and custom rules. Never run playbooks against hosts; `--syntax-check` or `--check` only for a maintainer PR and as the project interaction guide allows.
+
+## Correctness
+Idempotency: `command`/`shell` without `creates`/`removes`/`changed_when`; FQCN module names; `become` scope; handler names that `notify` no longer matches; variable precedence surprises (role defaults vs vars vs group_vars vs extra vars); `when` on bare variables or string booleans; `loop` vs `with_*`; Jinja2 templating and escaping in `.j2`; check-mode support; `serial` for rolling changes.
+
+## Failure paths
+`ignore_errors` hiding failures; wrong `failed_when`/`changed_when`; `block`/`rescue`/`always`; `retries`/`until` without a sensible bound; `any_errors_fatal` and `max_fail_percentage` on rolling plays.
+
+## Contracts
+Role `defaults/`, `vars/`, and `meta/argument_specs.yml` are the role's API: renamed variables break playbooks; inventory group names and tags used by other plays or CI.
+
+## Tests
+Molecule scenarios, `--check --diff` runs, `ansible-test` for collections; otherwise name the manual check (a check-mode run against a staging inventory).
+
+## Security
+`no_log: true` on tasks handling secrets; secrets in plain vars instead of Vault; file and directory modes/owners; `validate:` on files such as sudoers; variables interpolated into `shell`/`command`; `get_url` without `checksum`; `validate_certs: false`; broad `become_user`.
+
+## Clarity
+Task names describe the outcome; README and `argument_specs` match the defaults.

.agents/skills/ardan-pr-review/references/langs/ci.md

@@ -0,0 +1,24 @@
+# CI pipeline idioms
+
+Covers GitHub Actions, GitLab CI, Jenkins, CircleCI, Azure Pipelines, Bitbucket, Buildkite, Tekton, Drone, Travis, Cloud Build, Woodpecker — `stack.tsv` names the system in the marker column. Apply the general rules below in that system's syntax.
+
+**Tools:** `rg`. **Checks:** `actionlint` and `zizmor` for GitHub Actions; other systems have no offline linter — review by reading (repo-defined validators arrive as extra checks when approved, for a maintainer PR only). Never trigger a pipeline.
+
+## Correctness
+Triggers run where intended (PRs, forks, tags, branches, path filters, `rules`/`only`/`except`, `when`); job ordering (`needs`, stages, `dependsOn`); matrix coverage; cache keys that miss inputs or poison builds; artifact names and paths between jobs; concurrency groups and cancel-in-progress; `if:` expressions (string vs boolean, `success()` vs `always()`); shell steps quoting and `set -euo pipefail`; reusable workflow / template inputs.
+
+## Failure paths
+`continue-on-error`, `allow_failure`, `|| true`, `catchError` hiding failures; retries masking flakes; missing timeouts; post/cleanup steps that can't fail the build when they should.
+
+## Contracts
+Job and workflow names used as required status checks (renaming breaks branch protection); reusable workflow / template / shared-library inputs, outputs, and secrets; artifact and image names consumed downstream.
+
+## Tests
+No automated path: say so and name the manual check (a run on a test branch, `act`, `gitlab-ci-local`, Jenkins replay).
+
+## Security
+- GitHub Actions: `pull_request_target` or `workflow_run` checking out PR code (pwn request); `${{ github.event.* }}` interpolated into `run:` (script injection); third-party actions not pinned to a full SHA; `permissions:` missing or broader than needed (defaults to write); secrets reachable from fork PRs; OIDC trust conditions too broad; self-hosted runners on public repos; cache poisoning across trust levels.
+- GitLab/Jenkins/others: unprotected variables exposed to MR/fork pipelines; credentials echoed or interpolated into `sh`; Groovy `sh "${params.X}"` injection; script approvals and sandbox escapes; agents/runners shared across trust levels.
+
+## Clarity
+Step names describe what they do; comments on non-obvious conditions.

.agents/skills/ardan-pr-review/references/langs/docker.md

@@ -0,0 +1,21 @@
+# Dockerfile / Compose idioms
+
+**Tools:** `rg`. **Checks:** `hadolint` (Dockerfiles), `iac-scan`. No image is built or run by the checks.
+
+## Correctness
+Base image pinned by tag and digest; multi-stage builds copying the right artifacts; layer order vs build cache; `.dockerignore` keeping secrets and build output out of the context; exec-form `ENTRYPOINT`/`CMD` (signals reach PID 1); `apt-get update` and `install` in one `RUN` with `--no-install-recommends` and cleanup; `ARG` vs `ENV` scope. Compose: `depends_on` with health conditions, restart policies, volume and port mappings, env files.
+
+## Failure paths
+`SHELL ["/bin/sh", "-o", "pipefail", "-c"]` (or bash) when a `RUN` pipes; `HEALTHCHECK`; entrypoint scripts with `set -eu` and real exit codes.
+
+## Contracts
+Exposed ports, environment variable names, volume paths, entrypoint arguments, and the image name/tag scheme are the image's API for deployments and Compose files.
+
+## Tests
+Container structure tests or a CI build; otherwise name the manual check (build, run, hit the health endpoint).
+
+## Security
+`USER` non-root; no secrets in `ARG`/`ENV`/`COPY` or layers (use build secrets); `ADD` from URLs; `curl | sh`; `latest` tags; minimal base; setuid binaries; Compose `privileged`, host network, or Docker socket mounts.
+
+## Clarity
+Comments explain non-obvious build steps; labels (`org.opencontainers.image.*`) are accurate.

.agents/skills/ardan-pr-review/references/langs/go.md

@@ -0,0 +1,33 @@
+# Go idioms
+
+**Tools:** `gopls` MCP (`go_symbol_references`, `go_search`, `go_file_context`), else `gopls`/`go doc`, then `rg`. Skills: `use-modern-go` on every `.go` file; `ardan-service-architecture` on files with a `layer` (`app`, `business`, `storage`, `foundation`) — `references/types.md` for boundaries, `branching.md` for conditionals, `events.md` for domain events, `extensions.md` only when extension wiring is touched.
+
+**Checks** (`checks/summary.tsv`; package-wide tools report on whole packages, only in-scope hits count):
+- `go-build`, `go-vet`, `staticcheck`, `golangci-lint` — each hit is a finding at the Gate its effect warrants. (For an untrusted change `golangci-lint` runs with `--no-config`: default linters, since the repo's config can load custom linter plugins.)
+- `checklocks` — a violation of an existing `+checklocks`/`+checkatomic` annotation is a data race (G4). "may require checklocks annotation for X" names the lock guarding those fields: verify in-scope code holds it on every access.
+- `nilaway` — candidate nil dereferences; confirm the nil flow before reporting, else Open check.
+- `go-fix` — modernizer diff on in-scope lines: G1 (G2 where `use-modern-go` makes it a rule). `gofmt`, `go-mod-tidy` — G2.
+- `apidiff` — incompatible exported-API change: confirm every caller was updated; an unannounced break of a library package is G3 (Boundary Keeper).
+- `errcheck` — a finding only when the lost error matters (failed write, commit, close of a written file); a dropped `Close` on a read-only body is G1 at most.
+- `deadcode` — unreachable functions; an exported function of a library package may be public API.
+
+## Correctness
+`context` stored or not propagated, cancellation ignored; nil derefs; slice/map bounds; data races; leaked goroutines, rows, bodies, files; wrong transaction scope; surprising zero values. Architecture: primitives at API/DB edges, strong `business/types/*` in Business, crossings via `toBus<Type>` / `fromBus<Type>Response` / `toDB<Type>`, cross-cutting concerns via the `ExtBusiness`/`Extension` seam, shallow branching.
+
+## Failure paths
+Errors checked, wrapped with `%w`, matched with `errors.Is/As`, not `_`-discarded when they matter; overwritten before use; zero value returned after failure; `defer` cleanup failures that matter; goroutines with no error path; row-scan and iterator errors.
+
+## Contracts
+The type-boundary rule: API request/response structs use primitives; DB rows use primitive/native DB shapes; Business uses `business/types/*` strong types; every crossing goes through a named converter; nothing is assigned directly across App ↔ Business ↔ Storage. Storage is private to its domain: a `*bus` may import another domain's `*bus`, but nothing outside domain `<y>` imports `<y>`'s store or touches its tables; the App layer owns cross-domain transactions and every tx-bound `*bus` passes the tx to the `*bus` packages it depends on.
+- Check: primitives leaking into Business, strong types in API/DB structs, missing converters, validation with no owner; imports of another domain's store; SQL in `<x>db` reading or joining another domain's tables; a `*bus` beginning a transaction, or `NewWithTx` not rebinding a dependency; domain events defined outside the publisher's `<y>bus/event.go`, handlers not binding to the passed tx, external side effects inside a handler.
+- Types: exported vs unexported fields, constructors and their validation, zero-value meaning, pointer vs value receivers, mutators keeping invariants, small consumer-owned interfaces, needless interfaces/generics, aliases that erase meaning.
+- G4: layer-boundary or storage-isolation violation, constructible invalid Business state, converter bypass. Extra fields: `Type: <Name> (layer)`, `Crossing: toBus | fromBusResponse | toDB | none`.
+
+## Tests
+Table-driven tests with named subtests; `errors.Is/As` on error paths; negative, validation, boundary, cancellation, timeout, transaction and store-failure cases; HTTP request/response; converters; strong-type parsing. For a maintainer PR tests ran with `-race`: a `WARNING: DATA RACE` whose stack touches in-scope code is G4 — quote the two conflicting frames; prove a point with one `go test -run TestX ./pkg`. Otherwise `go test` didn't run.
+
+## Security
+`gosec`, `govulncheck` rows; trace taint from request/env/file to sink with `gopls`; `database/sql` string building, `exec.Command` with a shell, `html/template` vs `text/template`, `InsecureSkipVerify`, `math/rand` for tokens.
+
+## Clarity
+Exported identifiers documented where package style requires, starting with the identifier's name; comments on converters and strong types match the layer rules. Branch chains → default-first assignment or a naked switch (`branching.md`). Don't flatten layer converters or extension seams.