git:20260917.8b7632b to git:20260918.5f3c91f

- | 🌐 **Behaviour verification** | `mergecraft verify-behavior` writes a versioned report; `mergecraft review --verification-report` consumes it (fenced, separate from code findings; blocked stays visible; no report leaves the review unchanged); trusted-tier only; optional `mergecraft[browser]` extra — [`docs/verify-behavior.md`](docs/verify-behavior.md) |

-

- **Terminal verdict (default: enforce).** A run without a validated

- `submit_review_verdict` reports `inconclusive`. Set `gates.terminal_verdict: shadow`

- in `.mergecraft/config.yaml` to log diagnostics only.

-

- ## Authentication

-

- | Provider | Subscription (recommended) | API key | Recommended model |

- |----------|-----------------------------|---------|-------------------|

- | Anthropic Claude | `mergecraft provider auth anthropic --scope github` → indexed credentials | `ANTHROPIC_API_KEY` | `anthropic/claude-sonnet` |

- | OpenAI Codex | `mergecraft provider auth openai --scope github` → indexed credentials | `OPENAI_API_KEY` | `openai/gpt-5.3-codex` |

- | Google Gemini | `mergecraft provider auth google --scope github` → indexed credentials | `GEMINI_API_KEY` | `google/gemini-3.1-pro-preview` |

- | Nous Portal | — | `mergecraft provider auth nous --scope github` → indexed credentials; `NOUS_API_KEY` remains a legacy input | `nous/deepseek/deepseek-v4-flash` |

- | Tencent TokenHub | — | `mergecraft provider auth tokenhub --scope github` → indexed credentials | `tokenhub/hy3` |

- | MiniMax | — | `mergecraft provider auth minimax --scope github` → indexed credentials | `minimax/MiniMax-M3` |

- | Cursor Cloud | `mergecraft provider auth cursor --scope github` → indexed credentials | `CURSOR_API_KEY` | `cursor/cloud-agent` |

- | OpenAI-compatible (custom) | — | `MERGECRAFT_CUSTOM_PROVIDER_BASE_URL` + `MERGECRAFT_CUSTOM_PROVIDER_API_KEY` | `<your-prefix>/<your-model>` |

- | [LiteLLM](https://docs.litellm.ai/) proxy | — | same custom-provider env pair — [`examples/config-litellm.yaml`](examples/config-litellm.yaml) | `default/<model>` or `<label>/<model>` |

- | Logfire tracing | `mergecraft auth logfire` | see [`docs/TRACING.md`](docs/TRACING.md) | — |

-

- Custom OpenAI-compatible endpoints (including [LiteLLM](https://docs.litellm.ai/)

- proxies — no native `litellm` provider id), multi-provider indexed env vars, and

- `model:` chain semantics: [`docs/authentication.md`](docs/authentication.md).

-

- When `harness:` is unset, mergeCraft infers the runtime from the model slug — see

- [`docs/authentication.md`](docs/authentication.md) and

- [`docs/compatibility-matrix.md`](docs/compatibility-matrix.md).

-

- ## Docs

-

- | Doc | What it covers |

- |-----|----------------|

- | [`AGENTS.md`](AGENTS.md) | Agent entry point — consumer setup and contributing |

- | [`docs/glossary.md`](docs/glossary.md) | Plain-language definitions for landing-page terms |

- | [`docs/install.md`](docs/install.md) | Action vs CLI vs Docker install paths |

- | [`docs/authentication.md`](docs/authentication.md) | Providers, custom gateways, model fallback chains |

- | [`docs/agent-roster.md`](docs/agent-roster.md) | Agent roster, `provider status`, multi-reviewer merge, trust snapshot |

- | [`docs/workflows.md`](docs/workflows.md) | Examples 2–6, trust tiers, `pull_request_target` gotchas |

- | [`docs/cli.md`](docs/cli.md) | Full `mergecraft` command reference |

- | [`docs/verify-behavior.md`](docs/verify-behavior.md) | `mergecraft verify-behavior` and `review --verification-report`: flags, trust rule, artifact layout |

- | [`docs/trust-policy.md`](docs/trust-policy.md) | Operator trust knobs (`selfReview`, `agentSandbox`) |

- | [`docs/action-reference.md`](docs/action-reference.md) | Every Action `with:` input and output |

- | [`docs/EXIT-CODES.md`](docs/EXIT-CODES.md) | Contractual CLI exit codes |

- | [`REVIEW-CHECKS.md`](REVIEW-CHECKS.md) | Every check a review applies — lenses, gates, grading |

- | [`docs/ANALYZERS.md`](docs/ANALYZERS.md) | Analyzer catalog, trust tiers, SARIF upload |

- | [`docs/compatibility-matrix.md`](docs/compatibility-matrix.md) | Supported events, agents, providers, shell/push modes |

- | [`SECURITY.md`](SECURITY.md) | Threat model, trust tiers, reporting |

- | [`docs/`](docs/) | Full generated index |

-

- ## Development

-

- ```bash

- make setup # uv sync + pre-commit

- make lint # ruff

- make typecheck # mypy strict

- make test # pytest

- make ci # full gate

- ```

-

- See [CONTRIBUTING.md](CONTRIBUTING.md) for contributor workflow.

-

- ## License

-

- MIT — see [LICENSE](LICENSE).

-

- ===== FILE: AGENTS.md =====

-

- # AGENTS.md — guidance for AI agents

-

- Cross-vendor entry point for agents setting up mergeCraft in a **consumer** repo

- or contributing to **this** repository. Read [`README.md`](README.md) for the

- human landing page; use [`skills/mergecraft/SKILL.md`](skills/mergecraft/SKILL.md)

- for a compact setup checklist and CLI reference.

-

- ## Setup mergeCraft in a consumer repo

-

- Use this when asked to add AI PR review to another repository.

-

- 1. **Prerequisites** — Python **3.11+** ([`docs/dev/python-version-floor.md`](docs/dev/python-version-floor.md)),

- [uv](https://docs.astral.sh/uv/), and an authenticated [`gh`](https://cli.github.com)

- CLI. If Python 3.11+ is unavailable locally, use the Docker Action path only

- ([`docs/install.md`](docs/install.md)) — no local CLI install required for CI.

- 2. **Install the CLI** (PyPI is not published yet):

-

- ```bash

- uv tool install "merge-craft @ git+https://github.com/alexhawat/mergeCraft"

- mergecraft --install-completion # optional shell completion

- ```

-

- 3. **Scaffold config and workflow** — in the consumer repo root:

-

- ```bash

- mergecraft init

- ```

-

- 4. **Authentication — STOP here.** Interactive login is required. Ask the human

- to run `mergecraft provider list` and authenticate a configured label or id.

- For local evaluation use `--scope local`; for Actions adoption, examples

- below assume these provider labels were configured by `init`:

-

- ```bash

- mergecraft provider auth anthropic --scope github # Claude Pro/Max subscription

- mergecraft provider auth openai --scope github # ChatGPT Plus/Pro/Team/Enterprise

- ```

-

- Other providers: [`docs/authentication.md`](docs/authentication.md). **Never**

- invent, paste, or commit credentials, tokens, secrets, or `.env` files. Each

- `mergecraft provider auth … --scope github` stores a GitHub Actions secret via `gh secret set` — hand

- that step to the human when interactive auth is required.

- 5. **Commit only** `.mergecraft/config.yaml` and `.github/workflows/mergecraft.yml`

- on a new branch. Do not commit secrets.

- 6. **Trigger a review** — open a pull request, comment `@mergecraft review`, or

- run the workflow via `workflow_dispatch`. Local/offline review uses

- **`mergecraft review`** (not `diff-review`, which is a deprecated alias that

- emits one stderr warning per invocation).

-

- ## Setup with Grok Bot

-

- Use this when a Grok Bot (including the in-account **mergeCraft** bot) is

- asked to add mergeCraft to another repository.

-

- Grok Bot does **not** read `.agents/skills/` in a consumer repo. Install the

- generated skill from [`skills/grok-bot/mergecraft/SKILL.md`](skills/grok-bot/mergecraft/SKILL.md)

- as a Grok Bot user skill (Settings → Plugins → Yours; enable it for the Bot).

-

- Follow the same consumer flow as other agents:

-

- 1. **`uv tool install`** the CLI (see above).

- 2. In the **consumer repo root** (never in the mergeCraft source tree), run

- **`mergecraft init`**.

- 3. **STOP for authentication.** Ask the human to run exactly one

- `mergecraft provider auth … --scope github` command. **Never** invent, paste, or commit credentials,

- tokens, secrets, or `.env` files.

- 4. After the human confirms the GitHub Actions secret is stored, you may run

- **`mergecraft doctor`** and **`mergecraft review`** as needed.

- 5. Commit only `.mergecraft/config.yaml` and `.github/workflows/mergecraft.yml`

- on a new branch in the consumer repo.

-

- ## Working on this repo (development)

-

- mergeCraft itself: Python **3.11+**, managed with uv, recurring commands via **Make**

- only (see [`CONTRIBUTING.md`](CONTRIBUTING.md) and

- [`docs/_standards/coding-standards.md`](docs/_standards/coding-standards.md)).

-

- ```bash

- make setup # uv sync --extra dev + pre-commit

- make lint # ruff + format + loguru-only

- make typecheck # mypy strict + pyright pass

- make test # unit tests (not integration)

- make ci # full pre-merge gate

- ```

-

- - **Source:** `src/mergecraft/` — `cli/`, `agents/`, `analyzers/`, `mcp/`, `action/`, `config/`

- - **Docs:** `docs/`; review checks: [`REVIEW-CHECKS.md`](REVIEW-CHECKS.md)

- - **Review behaviour:** read [`docs/REVIEW-DOCTRINE.md`](docs/REVIEW-DOCTRINE.md) before

- editing review logic under `modes/`, `agents/`, or `analyzers/`

- - **Examples:** generated under `examples/` — edit templates in `scripts/`, not generated files

- - **MCP:** `mergecraft mcp serve` (HTTP, Bearer-required per-run token on an ephemeral port;

- reviewer role at `/mcp/reviewer`) and `mergecraft mcp list` — see [`docs/cli.md`](docs/cli.md)

-

- **Optional local overrides**

-

- On hosts without Linux namespace isolation (`unshare`), mergeCraft refuses to run the MCP

- shell tool and may refuse root outside the Action image. For local debugging only:

-

- - `MERGECRAFT_ALLOW_UNSANDBOXED_SHELL=1` — allow the unsandboxed shell fallback when PID

- namespace isolation is unavailable.

- - `MERGECRAFT_ALLOW_ROOT=1` — allow running as root outside the Action container image.

- - `MERGECRAFT_PROBE_ALLOW_SUDO=1` — allow the isolation probe to retry with `sudo`

- when `CI` is unset (local capability probing only).

-

- Do not set these in CI or production workflows.

-

- ## Rules for agents

-

- - Do not weaken trust-tier or fail-closed security behaviour

- ([`docs/workflows.md`](docs/workflows.md), [`SECURITY.md`](SECURITY.md)).

- - Do not add entries to `evidence/` (slated for deletion).

- - Conventional Commits; subject ≤ 72 characters; no `--no-verify` unless the operator allows it.

-

- ===== FILE: REVIEW-CHECKS.md =====

-

- # What mergecraft checks for

-

- > **Agent-facing artifact:** `.github/skills/code-review/` is what mergeCraft injects

- > into the reviewing agent at runtime; this file is the human-facing catalog of the

- > same checks.

-

- > **Doc status (W7 + catalog C6):** §2 describes the analyzer platform and the expanded

- > P0–P3 catalog. Long-tail tools default to **disabled** unless repo config or detection

- > enables them.

-

- Every check mergecraft applies when it reviews a pull request, grouped by what it is looking at.

-

- A quick orientation before the lists:

-

- - Most of these are **judgment checks** carried out by the reviewing agent, not scripted rules. There is no rule engine — the behavior lives in the `Review` and `IncrementalReview` mode prompts in [`src/mergecraft/modes/Review.py`](src/mergecraft/modes/Review.py) and [`src/mergecraft/modes/IncrementalReview.py`](src/mergecraft/modes/IncrementalReview.py).

- - **Mechanical evidence** comes from two layers: your repo's own gates (`staticChecks` / Makefile targets via `run_static_checks`) and mergeCraft's **catalog analyzers** (`run_analyzers`). Only **`failed`** gate status and **verified** analyzer findings become review signal — everything else is reported as skipped.

- - Groups 1–3 are the ones that produce findings. Groups 4–8 govern how findings are graded, placed, filtered, and formatted — they are why the review stays short.

-

- ## Contents

-

- 1. [Code correctness and risk](#1-code-correctness-and-risk) — the review lenses

- 2. [Analyzers](#2-analyzers) — catalog tools + repo gates

- 3. [Pull request hygiene](#3-pull-request-hygiene) — the pre-merge checks table

- 4. [Diff coverage](#4-diff-coverage)

- 5. [Finding grading](#5-finding-grading)

- 6. [Findings that get dropped](#6-findings-that-get-dropped)

- 7. [Memory across runs](#7-memory-across-runs)

- 8. [Output shape](#8-output-shape)

- 9. [Address-reviews checks](#9-address-reviews-checks)

- 10. [Trajectory checks](#10-trajectory-checks-43-49)

- 11. [Deterministic run record](#11-deterministic-run-record-plan-12)

-

- ---

-

- ## Mechanical evidence — what counts (#41, W2.5)

-

- Mechanical evidence is what the merge-evidence packet calls **structural**

- — typed `Finding`s, deterministic gate outcomes, and CI check-suite

- results. It is the *only* category of evidence that can move the merge

- verdict; everything else is advisory. Concretely:

-

- - **Typed `Finding`s** — emitted by the analyzer catalog (`Finding` from

- `mergecraft.analyzers.finding`, `extra="forbid"`). Each finding carries

- `tool`, `rule_id`, `category`, `severity`, `confidence`, `path`,

- `start_line`/`end_line`, `fingerprint`, `evidence: list[str]`,

- `introduced_by_pr`, `source`, `scope`, `cluster_id`. Findings are the

- authoritative structural input to `decide_approval()`. Only `scope="change"`

- findings can block; `scope="run"` rows are advisory and partition into the

- packet's `run_health` section at assembly time (plan 12).

- - **`DeterministicCheck` rows** — one per declared `staticChecks` or

- discovered Makefile target. Status is one of five: `passed`, `failed`,

- `timed_out`, `unavailable`, `declared-but-cannot-run`. **Only

- `failed`** is a negative finding; **only `passed`** is a positive

- signal. The other three are honest skips, never silent passes.

- - **CI check-runs** — raw `check_suite` data from `mcp/check_runs.py`

- plus the cluster/blame/flaky annotations from `src/mergecraft/ci/`.

- - **The agent's `approved` boolean** — **NOT** mechanical evidence. It

- is recorded on the packet as `self_assessment` and is advisory only;

- a self-assessment-only run cannot reach `auto_merge` (the #41 hard

- rule, pinned by `tests/evidence/test_self_assessment.py`).

-

- What does *not* count as mechanical evidence, even when it appears in a

- check-run summary or in the agent's prose:

-

- - The agent's review narrative — `ApprovalRecord.would_approve`,

- `result.output`, anything the model wrote.

- - PR title / body / comment text (even when fenced and unfenced by

- trust tier) — see the `Trust tiers and contributor weight` section in

- `docs/REVIEW-DOCTRINE.md`.

- - A "green" status with no underlying typed finding — `unavailable` /

- `declared-but-cannot-run` / `timed_out` are explicit and visible; the

- absence of evidence is itself evidence the verdict must surface.

-

- A behaviour-verification report (from `mergecraft verify-behavior`, consumed

- via `--verification-report`) is **not** a typed `Finding` and is **not**

- mechanical evidence for the approval gate. It is a separate artifact: page

- text and console output the change under review can control, so it is fenced

- before any prompt. Treat it as evidence about the running app, not a

- substitute for reading the diff. `skipped` and `blocked` are not a pass —

- `blocked` names the missing input and stays visible; omitting the report

- leaves the review unchanged.

-

- The merge-evidence packet's `decision` row is computed by

- `mergecraft.agents.gates.decide_approval(findings, *, run_succeeded,

- tier)` from these structural inputs. When the packet is given directly

- to `decide_approval(packet, …)`, the explicit `Decision` row on the

- packet wins over every other signal — including the recorded

- `self_assessment`.

-

- ## Terminal verdict vs structural verdict (VP2)

-

- A review run is not complete until the agent records a **terminal

- submission** through `submit_review_verdict`. Provider success, review

- prose, and `create_pull_request_review` publication are separate acts:

-

- - **Agent verdict** — the model's `approve` / `request_changes` choice,

- summary, and structured findings submitted through the typed MCP tool.

- This is the only signal that answers "did a review happen on this

- attempt?"

- - **Structural verdict** — what `decide_approval` computes from typed

- findings, `run_succeeded`, and trust tier. Narrative output and

- `ApprovalRecord.would_approve` are advisory only and never override a

- confirmed blocker.

-

- **Schema vs semantic validation.** The tool rejects unknown fields and

- invalid verdict enums at parse time. After schema validation,

- `validate_submission` applies semantic rules server-side: `request_changes`

- with zero findings, `approve` over a verifier-confirmed Critical/Major

- blocker, and `approve` while a required deterministic gate failed are all

- rejected with a typed `rejection_reason`. A rejected submission does not

- set `terminal_submission_received` — the attempt is fallback-eligible and

- maps to `RunOutcome.inconclusive`, same as no submission at all.

-

- **Why prose is not authoritative.** Text such as "LGTM" in `result.output`

- has never been an input to `decide_approval` and cannot approve a pull

- request. A run whose provider returned successfully but never called

- `submit_review_verdict` now reports `inconclusive` (`neutral` check

- conclusion) instead of `passed`.

-

- **Fallback interaction.** Semantic fallback advances when

- `terminal_submission_received` is false — whether because no submission

- was recorded or because the validator rejected one. A valid

- `request_changes` verdict with confirmed findings is a usable result and

- does not trigger fallback.

-

- ## 1. Code correctness and risk

-

- The reviewer reads the whole diff itself, then picks the **lenses** the PR actually warrants and investigates each as a falsifiable question — optionally dispatching a `mergecraft-reviewer` subagent per lens so they run in parallel. Nothing here is a fixed pass; a docs-only diff gets none of it.

-

- Each review round records which lenses were **selected**, which were **skipped** (with reasons from deterministic routing), and which were **actually dispatched**. That set is written into the review metadata HTML comment and the merge-evidence packet so the next round — and incremental complement routing in a follow-up review — can read what already ran without re-deriving it from prose.

-

- **Run lifecycle (S1)** — a failed or timed-out trusted-tier `setupScript` yields `RunOutcome.inconclusive` (neutral check conclusion), not a review. An under-provisioned tree never receives a review verdict. See [`docs/config-failure-policy.md`](docs/config-failure-policy.md#setup-script-failures-s1--d5--d10--f6) for the policy table and operator checklist.

-

- **Always in play**

-

- - **Correctness and invariants** — bugs, races, error handling, edge cases, state-machine boundaries.

- - **Data integrity and atomicity** — for any diff that writes persistent state: is the write ordered after the thing it records is confirmed, or before? does failing halfway leave a half-committed state with no rollback? is a retry idempotent, or does it double-apply?

- - **Impact** — stale references left in code, tests, docs, configs, or UI after a rename or removal.

- - **Copy vs code** — does every human-readable string still match what the code does? Help text, menu labels, error messages, `--help` output, README and doc claims, and the PR description's own promises.

- - **Holistic** — does the PR make sense as a whole? Symmetric flows: a delete for every create, a rollback for every migration.

-

- **Picked when the diff warrants it**

-

- - **Security** — new endpoints, authorization, input validation, secret handling, replay / CSRF / injection, cross-tenant isolation.

- - **Privilege drop ordering** — for a diff where a privileged process (root, before a `setpriv`/`sudo -u`/`su`/container-user-switch step) creates a file or directory that a later, lower-privileged process must then read or write: does the write land only after ownership is fixed for the dropped-privilege user, or does the privileged process's plain `mkdir()`/file write leave the path owned by the wrong uid? Ownership follows the *creating* process, not the parent directory's owner and not a later chmod/chown applied only to the parent — this is how mergeCraft shipped `Permission denied` bugs against its own `$HOME` and `$CODEX_HOME`/`.gemini`/`.claude` writes twice in production.

- - **Performance** — N+1 queries, hot-path allocation, latency budgets, index coverage.

- - **Test integrity** — meaningful coverage for the changed behavior, deterministic, no shared-state pollution.

- - **User journey** — for UX-touching flows, walking the happy path and the failure modes as a user.

- - **Operational readiness** — observability, alerting, forward and rollback migrations, feature flags, on-call burden.

- - **Integration and cross-cutting** — API contracts between modules, backward compatibility of public surfaces, multi-service ordering.

- - **Research-validated assumptions** — third-party API contracts, SDK semantics, framework directives, version-gated behavior. Only when the PR's correctness *depends* on the contract behaving a certain way, and the reviewer must cite source URLs.

- - **Subsystem lenses** — invented per PR for high-stakes domains: auth, billing, payments, schema migration, webhooks, secrets, RBAC, multi-tenant isolation, cron/scheduling. Preferred over the generic equivalent, because "the billing lens" primes for double-charges and refund races in a way "correctness on billing code" does not.

-

- **Non-anchored concerns** — deliberately hunted for after the line-anchored findings, since these have no line to point at:

-

- - Deletion or cleanup plans for code the diff replaces or shadows.

- - Rollout sequencing — what happens to in-flight state during deploy or revert.

- - Coverage gaps the diff implies but does not add.

- - Scope questions only a human can answer (is the legacy path going away, or is this a long-term dual track?).

- - Architectural risks the diff opens up that are not a single-line bug.

-

- ## 2. Analyzers

-

- Deterministic evidence from catalog tools and your repo's own gates. The reviewer calls `run_analyzers` (catalog) and `run_static_checks` (repo gates) early in Review / IncrementalReview — results feed the **Analyzers** and **Mechanical gates** pre-merge rows and may become inline findings.

-

- ### Repo mechanical gates (`run_static_checks`)

-

- Your repo's own gate — unchanged from prior mergeCraft behavior:

-

- - **Declared gates** — `staticChecks` in `.mergecraft/config.yaml`. `{files}` expands to changed paths; `suffixes` skips when no matching file changed.

- - **Discovered gates** — with nothing declared, mergecraft looks for `lint`, `format-check`, `typecheck`, and `ci-static` Makefile targets. Skipped when `make` is not installed.

- - **Nothing found** → skipped. mergecraft will **not** substitute a linter of its own (`except A, B:` is legal on Python 3.14 and a syntax error on 3.13 — version mismatch manufactures false positives).

-

- Each gate returns one of six statuses; only **`failed`** is a finding:

-

- | Status | Meaning |

- |---|---|

- | `passed` | ran, exit 0 |

- | `failed` | ran, non-zero exit — a finding |

- | `timed_out` | exceeded the per-gate timeout |

- | `unavailable` | executable not installed — judged nothing |

- | `declared-but-cannot-run` | gate is declared in config but this environment cannot execute it (for example `shell: disabled` on a pull-request event) — judged nothing, but the gate is visible instead of silently omitted |

- | `satisfied-by-ci` | the gate did not run here, but a check run your repo **declared** as proof of it passed on this commit — green, with the check run named |

-

- When `staticChecks` are configured but every gate is `unavailable` or `declared-but-cannot-run`, `run_static_checks` returns `ran: false` with an explicit reason and one row per configured gate so the **Mechanical gates** pre-merge row can report skipped instead of implying the repo has no gates.

-

- #### Reusing your CI as gate evidence (`ciEvidence`, #36)

-

- The Action image usually has no `make`, no repo venv, and none of your pinned toolchains — so a gate reports `unavailable` even when your own CI just proved it on the same commit. Declare the mapping and that finished CI stands in:

-

- ```yaml

- ciEvidence:

- gates:

- lint: Verify (drift gates) # <gate name>: <exact GitHub check-run name>

- ```

-

- - **Declared only.** A check run merely *named* like a gate proves nothing — a pull request can add a workflow with any name it likes. With no `ciEvidence` block mergeCraft never reads your check runs at all.

- - **Only green substitutes.** A declared check run that passed rewrites that gate's row to `satisfied-by-ci`, replacing the `unavailable` row rather than adding a second one. A declared check run that **failed** leaves the honest row in place and is reported as a CI finding — the report never claims a green gate on red evidence.

- - **A gate that actually ran here always wins.** CI cannot overwrite a verdict mergeCraft produced against this diff.

- - **Best effort.** No head SHA, no declared mapping, or a GitHub API error → the gate report is exactly what it would have been without the feature.

-

- ### Catalog analyzers (`run_analyzers`)

-

- Shipped catalog spans **P0 workflow/Docker gates** (actionlint, zizmor, ShellCheck, Hadolint),

- **repo-native language gates** (Ruff, type checkers, ESLint/Biome/Oxlint), **supply chain**

- (OSV-Scanner, Trivy, TruffleHog), **pattern scanners** (Semgrep/OpenGrep, ast-grep),

- **differential contracts** (oasdiff, Squawk, buf breaking), **agent security** (native YAML

- rules on MCP/skill manifests), and **P1–P3 long-tail manifests** (Go, Rust, IaC, SQL, PHP,

- Ruby, …) — each is YAML plus an existing parser, not bespoke adapter code.

-

- Reference: [`docs/ANALYZERS.md`](docs/ANALYZERS.md) (generated from manifests; CI-enforced).

- Contributor path: [`docs/CONTRIBUTING-ANALYZERS.md`](docs/CONTRIBUTING-ANALYZERS.md).

- Offline inspection: `mergecraft analyzers list|detect|run|explain|export --sarif|lock`.

-

- **Execution preference (D4):** `repo-native` → existing CI result → managed pinned binary → container → **skip with a named reason**. Skipped is skipped — never a finding, never a failed pre-merge row.

-

- **Trust tiers (D7):** `trusted` (same-repo PR, `workflow_dispatch`, offline `diff-review`) vs `untrusted` (fork PR / `pull_request_target` — no secrets, network deny-by-default, no PR-authored command construction; trusted-only manifests skip with reasons). Offline `diff-review` runs analyzers at trusted tier without shell.

-

- **`shell: disabled` (#35):** hardening the workflow no longer costs you the catalog. Repo-declared gates stay withheld — they run command strings the PR author controls — but mergeCraft's own **`managed` / `container` analyzers still run**, because their argv comes verbatim from a manifest mergeCraft ships and a repo-provided binary may not stand in for the pinned one. `repo-native` manifests are withheld, each with a named reason, since they exist to run the repo's own tool against the repo's own config. What a consumer sees on a `pull_request_target` + `shell: disabled` run: analyzer rows for the managed tools that matched the diff, `unavailable` rows naming why each other manifest was withheld, and the `staticChecks` gates reported as `declared-but-cannot-run`. The full runtime × shell × trust matrix is generated into [`docs/ANALYZERS.md`](docs/ANALYZERS.md).

-

- **Scoping (D6):** analyzers run on **head** by default; findings outside the diff hunks are dropped unless the path is an explicit exception (new file, dependency manifest, lockfile, workflow, migration). `introduced_by_pr: unknown` when no base run happened — never implied `true`.

-

- **Clustering (D12):** one defect from multiple tools publishes **one** finding with corroborating evidence and raised `confidence`.

-

- **Verification (D11):** Critical/Major analyzer hits are **hypotheses** until the read-only `mergecraft-verifier` subagent confirms, downgrades, or drops them. Drops write a reason under `## Withdrawn review findings (known non-issues)`.

-

- **Noise budget (D14):** inline analyzer slots cap at **8** (W0.2 measurement). Analyzer overflow lands in `### 🔧 Mechanical findings` (compact tool table). Agent overflow lands in `### 🗂 Deferred findings` with full finding text (non-blocking, server-appended). Agent findings win ties; Trivial/Low value never inline.

-

- **Lockfile (D24):** `.mergecraft/analyzers.lock` records resolved tool id, version, source, and SHA256; the pre-merge **Analyzers** row echoes the digest.

-

- ### CI pipeline intelligence (`analyze_ci_failures`)

-

- When CI failed on the PR head, mergeCraft calls `analyze_ci_failures`, which wraps `get_check_suite_logs` and normalizes failures behind `GitHubActionsProvider`.

-

- To discover a `check_suite_id` for a commit, call `list_check_runs` with the PR head SHA (or `get_check_suite` when you already have an id). Then pass that id to `get_check_suite_logs` or `analyze_ci_failures`.

-

- **What is read**

-

- - Failed workflow job/step names, exit codes, and redacted log excerpts

- - Retry attempt history and base-branch run fingerprints (when available)

- - PR diff paths for blame overlap

-

- **What is inferred**

-

- - Root-cause clustering by failure fingerprint (twelve shards from one broken import → one finding)

- - Flaky vs stable vs pre-existing classification from retry flips and base-branch evidence

- - PR attribution (`caused_by_pr` vs `probably_not_this_pr`) from diff overlap — never asserted without evidence

- - Truncation when more failures exist than the configured cap (default 3)

-

- **What is explicitly not claimed**

-

- - mergeCraft does not re-run CI, retry jobs, or push fix commits

- - Non-GitHub providers (CircleCI, GitLab, Azure) are honestly stubbed — no silent empty results

- - A failure outside the diff is **reported, not blamed** on the author

- - Flaky failures are named flaky rather than treated as the author's defect

-

- The review publishes `### 🚨 CI failures` with clustered root causes, flaky/blame verdicts, and redacted excerpts. The pre-merge **CI** row reports failure count, cluster count, flaky count, PR-attributed count, and whether truncation occurred. Inline CI comments may carry a one-click `suggestion` when the fix is a contained single-hunk edit; pushing a fix commit stays behind the existing `push` permission.

-

- **Recorded as evidence (#36).** Each clustered CI failure is also recorded as a `source: ci` finding on the run and carried into the [merge evidence packet](docs/REVIEW-DOCTRINE.md), keeping the blame verdict it was given: a failure attributed to this PR is `Major` / `introduced_by_pr: true`, while a flaky or pre-existing one is `Minor` / `introduced_by_pr: false`. Since every gate that consumes findings is monotone in blockers, that annotation is what makes "reported, not blamed" mechanical rather than a matter of wording — a flaky pipeline cannot block a clean pull request.

-

- **SARIF your CI already produced.** Naming artifacts under `ciEvidence.sarifArtifacts` lets the reviewer ingest their SARIF as CI findings through the same parser the analyzer catalog uses. Default is empty, in which case no artifact API call is made. Ingested results are reported at a non-blocking severity with `introduced_by_pr: unknown` — SARIF from another pipeline describes the tree, not this diff.

-

- Implementation: [`src/mergecraft/ci/intelligence.py`](src/mergecraft/ci/intelligence.py), [`src/mergecraft/ci/review.py`](src/mergecraft/ci/review.py), [`src/mergecraft/ci/cluster.py`](src/mergecraft/ci/cluster.py), [`src/mergecraft/mcp/ci_intelligence.py`](src/mergecraft/mcp/ci_intelligence.py), [`src/mergecraft/mcp/check_suite.py`](src/mergecraft/mcp/check_suite.py), [`src/mergecraft/mcp/check_runs.py`](src/mergecraft/mcp/check_runs.py).

-

- ## 3. Pull request hygiene

-

- Assertions about the pull request itself rather than its code. These always appear, as a small **Pre-merge checks** table at the top of the review body:

-

- - **Title** — does it name the main change? Flagged when it covers only part of the diff, or names something the diff doesn't do.

- - **Description** — does it explain what changed and why, and does every claim in it hold against the diff?

- - **Linked issues** — for each issue the PR closes, is every stated requirement actually covered?

- - **Scope** — does the diff do things neither the description nor a linked issue asked for? Out-of-scope paths get named.

- - **Mechanical gates** — the result from `run_static_checks` (repo gates).

- - **Analyzers** — the result from `run_analyzers` (catalog tools): how many ran, how many skipped (with reasons), lockfile digest.

- - **CI** — pipeline intelligence on failing check suites: failure count, cluster count, flaky count, PR-attributed count, and whether truncation occurred. Flaky or probably-not-this-PR failures are reported here, not blamed on the author.

-

- A flagged row here is fixed by editing the PR's title, body, or issue links — not its code — so these never also become inline comments. The one exception is a failing mechanical gate, which is a real code finding and is raised inline too.

-

- ## 4. Diff coverage

-

- Checks on the review process itself, so a review can't quietly skip half the PR:

-

- - The complete raw diff is read end-to-end, using the diff's table of contents as a coverage checklist.

- - On a re-review, an incremental patch covering only the commits since mergeCraft's last review scopes what is new — the full diff still establishes coverage.

- - A first submission that missed regions gets a one-time nudge listing the unread ranges.

- - Understanding is never delegated: subagents supply lens investigations, but the primary reviewer synthesizes and re-verifies every finding.

- - **Trivially skippable** — a single-word doc typo, whitespace-only changes, comment-only changes, lockfile or generated-code regeneration, a mechanical rename, a low-risk dependency patch bump.

- - **Looks trivial but is not** (small diff, big blast radius — never skipped):

- - any one-line change to SQL, regex, auth, billing, permission, or signature-verification code

- - flipping a feature-flag default, a default config value, or a retry/timeout constant

- - changing a money, tax, currency, or fee constant by any amount

- - changing an HTTP method, redirect URL, response code, or status enum

- - tightening or loosening a comparison operator (`<` ↔ `<=`, `==` ↔ `!=`)

- - renaming a public API surface

- - adding a new direct dependency

- - a "typo fix" in user-facing copy that changes meaning ("approved" → "denied")

- - a semantic one-liner buried in a formatting-only diff

-

- ### Blast-radius merge lanes

-

- The packet's blast-radius classification is an evidence-weighted policy signal,

- not an instruction to merge. `low` means the change is eligible for the

- auto-merge lane after required checks pass, `medium` means assisted review, and

- `high` means automatic merge is forbidden. These semantics do not enable or

- disable auto-merge; `autoMergeEnabled` remains `false`, and Batch D (#46) owns

- the separate mapping from evidence outcomes to workflow actions.

-

- ## 5. Finding grading

-

- Every surviving finding is graded on three independent axes before it is placed. The grade decides placement, so it isn't decoration.

-

- - **Category** — picked by where the *consequence* lands, not what the code looks like: Functional Correctness · Data Integrity & Atomicity · Security & Privacy · Stability & Availability · Performance & Scalability · Maintainability & Code Quality.

- - **Severity** — Critical (blocks merge) · Major (real fallout if shipped) · Minor (worth fixing, ships fine without) · Trivial (nit).

- - **Effort** — Quick win (contained, obvious) · Heavy lift (needs design, spans files, or has migration implications) · Low value (correct, not worth the churn).

-

- Then placement is mechanical:

-

- - `Trivial` **or** `Low value` → a bullet in the body's Nitpicks list, never an inline comment.

- - everything else → an inline comment at its line, tagged `_{category}_ | _{severity}_ | _{effort}_`, **unless** it overflowed the inline budget as an agent finding — then it lands in the non-blocking `### 🗂 Deferred findings` section (server-appended, full text, no inline anchor).

-

- **Collateral (RC11):** every `Critical` or `Major` finding names what else must move with the fix — callers, tests, docstrings, configs, or other files — in the finding's `collateral` list and in the inline comment body under an **Also update:** bullet list. Collateral is not required for `Minor` or `Trivial` findings. Any collateral claim about code the diff doesn't contain must ship with evidence; without evidence it is downgraded to a question or dropped (§6).

-

- The axes are also a sweep: a PR that writes persistent state with no Data Integrity & Atomicity finding gets one more look before the reviewer concludes there was nothing there.

-

- Values live in [`src/mergecraft/review_taxonomy.py`](src/mergecraft/review_taxonomy.py); a test asserts the prompt still names every one of them.

-

- ### Verification before publication

-

- Every `Critical` / `Major` finding is a hypothesis until a second, read-only agent

- (`mergecraft-verifier`) has read the cited code. That applied to analyzer and CI findings from

- the start; it now applies to the findings the reviewing agent wrote itself, which is the source

- most likely to be wrong.

-

- Before publishing, the reviewer hands its own `Critical` / `Major` findings to

- `verify_agent_findings`, which returns one dispatch brief per finding — the finding, its cited

- file, and the withdrawn-findings section. Three things bound the cost:

-

- - **Severity** — `Minor` and `Trivial` findings are never verified.

- - **Memory** — a finding whose fingerprint already appears under `## Withdrawn review findings` is

- skipped outright, not re-verified.

- - **Budget** — dispatches are capped at the repo's `review.verificationBudget` (default 24; `0` =

- no cap), spent on `Critical` before `Major`. Verification depth is independent of inline

- placement (`analyzers.inlineBudget`, default 8). Over-budget fingerprints are recorded in

- `skippedOverBudget` rather than silently dropped.

-

- Each verdict goes back through `record_finding_verdict`: **confirm** publishes as drafted,

- **downgrade** re-grades, and **drop** writes the verifier's reason under

- `## Withdrawn review findings` so the finding stays refuted on every later run.

-

- **The verifier is an LLM judge, and therefore a secondary signal.** It runs *after* the

- deterministic checks — the tools refuse to plan a dispatch or accept a verdict until analyzers or

- repo gates have had their turn — and it never overrules a tool result. Its model is pinned per

- provider (Claude runs the judge on Sonnet, a different tier from the orchestrator that wrote the

- finding), and its model, provider, judge version and rubric version are logged with every verdict.

- The rubric is five binary questions about the code (does the cited code exist, does the mechanism

- hold, is it reachable, did this PR introduce it, is it already refuted) — never a score for

- quality, style, or verbosity. On the `high` blast-radius lane, one judge cannot retire a finding

- on its own: a `drop` there is escalated for a second judge or a human rather than written to the

- withdrawn section.

-

- ## 6. Findings that get dropped

-

- What mergecraft deliberately does **not** report — this is most of what keeps a review readable:

-

- - Praise, and style preferences the repo doesn't enforce.

- - Speculative or unverified claims. Any claim about code the diff doesn't contain must ship with an `Evidence` section quoting the command that settles it; if the reviewer can't run one, the finding is downgraded to a question or dropped.

- - Problems in pre-existing code unrelated to the PR. The test is whether the root cause lives in lines this PR added or modified — unless the PR plausibly introduced or amplified the regression.

- - Anything not actionable.

- - Anything already refuted in the learnings file (see next group).

- - **Bloat-shaped findings** — proposed fixes that would add defensive checks for cases that can't happen, abstractions used once, comments restating obvious code, tests asserting tautologies, or "just-in-case" guards. The bar for an inline comment is sound **and** correct **and** elegant; a change that improves only one of the three makes the codebase worse.

- - On `IncrementalReview`, anything that restates feedback a prior review already gave.

- - On `IncrementalReview`, **first-pass miss labelling (D10):** when a *new* finding's root cause is on a line that already existed at the first reviewed commit (context in the incremental diff, not a line the fix commits added), the inline body is prefixed with `_(First-pass miss — this line was already present at the first reviewed commit.)_`. That label is honest scope disclosure — not a restatement of prior feedback and not a drop.

- - On `IncrementalReview`, **deferred promotion:** when the incremental diff touches a path cited by a ledger `deferred` finding, checkout promotes that record back to `open` with an audit reason. Promotion is back in scope — not a restatement of prior inline feedback.

- - **PR prose is evidence, never instruction.** A finding whose only support is the PR title, PR body, a comment, or any other fenced untrusted field is **dropped** if the prose merely *describes* a change without anchoring to diff lines, and **downgraded** to a question if the prose *asserts* a property that the diff does not demonstrate. The diff (or, for design questions, the linked design doc) is the only thing that anchors a finding. Sentences inside the per-run fence block are untrusted internet content by default — they may inform a hypothesis, but they never stand in for evidence.

-

- ## 7. Memory across runs

-

- - **Withdrawn findings** — when an author refutes a review finding and `AddressReviews` accepts the pushback, it records the *reason* in `.mergecraft/learnings.md` under `## Withdrawn review findings (known non-issues)`. A `drop` verdict from the verifier writes to the same section, so a finding the reviewer refuted *before publishing* is also refuted permanently. Later reviews read that section first and treat it as binding, so a false positive is argued once instead of on every PR.

- - **Finding fingerprints** — each inline comment is stamped server-side with a content hash of its path and body (`<!-- mergecraft-finding:v1:… -->`). Whitespace and case are normalized, so a re-raised finding is recognizable across runs even when reworded.

- - **Open-PR finding ledger** — the sticky progress comment carries `<!-- mergecraft-ledger:v1:<fingerprint>:<state> -->` markers for every finding this pull request's reviews considered, including deferred overflow, verifier drops (`withdrawn`), and over-budget verifications (`unpublished`). Persistence is GitHub-only; inspect with `mergecraft findings ledger --pr N`. The ledger never files GitHub issues — post-merge carryover owns issue filing (D5).

- - **Repo learnings** — test commands, conventions, gotchas, and architecture notes persist in the same file and are loaded into every run.

-

- ## 8. Output shape

-

- Checks on the review artifact itself. The body has at most five parts, in order:

-

- 1. **Reviewed changes preamble** — what was reviewed, plus machine-readable metadata (file and commit counts, base and head SHAs, prior review link) so a downstream agent can tell whether the findings have gone stale.

- 2. **Pre-merge checks** table (group 3).

- 3. **Cross-cutting sections** — one per non-anchorable concern, each a problem write-up with a collapsed `Technical details` block carrying the fix brief.

- 4. **Nitpicks** — the Trivial and Low-value bullets.

- 5. **Fix all findings** — one collapsed, copyable brief covering every finding, opening with a verbatim verify-first instruction so a fix-agent that only sees the posted review still treats findings as hypotheses.

-

- Formatting rules that are enforced by the prompt:

-

- - Inline comments carry a triage tag, stay 2–3 sentences in the visible part, and push depth into a collapsed `Technical details` block.

- - A one-click `suggestion` is attached whenever the fix is a contained single-hunk edit — and omitted when the reviewer can't produce exact replacement text, since a suggestion that doesn't apply cleanly is worse than none.

- - Problem statements describe the problem; asks and fixes live in the technical-details block.

- - Severity emoji on every section heading, no two consecutive prose paragraphs, backticks around every identifier, no repeated diff content, no line-count stats.

- - The opening callout tier (`[!CAUTION]`, `[!IMPORTANT]`, informational, or ✅) must match the author's actual next action — wrapping mergeable feedback in `[!IMPORTANT]` trains people to ignore it.

- - **Multi-reviewer provenance (`raised_by`).** When more than one reviewer binding

- runs, each finding may carry a `_Raised by: \`reviewer-id\`_` line in the

- published body. The field is **server-stamped at merge time** from the dispatch

- pairing `(reviewer_id, findings)` — it is **not** on the agent-facing

- `submit_review_verdict` schema (`additionalProperties: False`; an agent-supplied

- `raised_by` is rejected). Unknown provenance reads `unknown`, never the primary

- reviewer. `raised_by` is display and record only: it does not affect verdict,

- severity, dedup identity (`finding_key` stays `(path, body, line)`), or inline

- placement.

-

- ## 10. Trajectory checks (#43, #49)

-

- Everything above reads the **diff**. These eight checks read *how the run

- produced it* — the tool calls mergeCraft mediated. A diff can look clean while

- the process that produced it was not, and that is invisible to a diff review.

-

- | Check | Fires when | Severity |

- |---|---|---|

- | `changed-unread-file` | A file was modified that the run never read | Major |

- | `ignored-tool-error` | A tool call errored and that tool was never called again | Major |

- | `no-post-edit-verification` | Files were modified and nothing verifying ran *afterwards* | Major |

- | `repeated-tool-loop` | The same call, with identical arguments, three or more times | Minor |

- | `unresolved-failure` | A command reported failure and no later run of it passed | Critical |

- | `suspicious-broad-edit` | One run modified 25+ files | Minor |

- | `stale-assumption-after-failure` | A failed call was retried byte-identically with nothing read in between | Major |

- | `missing-completion-signal` | The run did work and never signalled completion | Minor |

-

- Each finding carries the severity above and a recommended action.

-

- **Silence on absent evidence.** mergeCraft only sees the calls it mediates, so a

- driver whose file reads never cross MCP produces a record with no reads — which

- is *unknown*, not *unread*. Every check that could fire on missing signal is

- gated on the record carrying that signal at all: `changed-unread-file` needs

- `read_coverage`, `missing-completion-signal` needs at least one recorded call. A

- check that fires on every run is noise, not a gate.

-

- **Run-scoped, never blocking (D2).** Trajectory checks stamp `scope="run"`,

- `source="trajectory"`, and `introduced_by_pr="false"`. They partition into the

- packet's `run_health` section and render under a separate collapsed heading in

- the deterministic run record. `blocking_findings()` drops them before severity

- grading — no severity, and no future check, makes a run-scoped finding fail a

- PR. There is no separate trajectory verdict and no second required check-run

- (D14).

-

- **They never crowd out code findings.** Inline slots go to code findings first;

- trajectory findings take only what is left and otherwise report in the body or

- the run-health section.

-

- ## 11. Deterministic run record (plan 12)

-

- Every run that resolves a PR number leaves exactly one authoritative sticky

- progress comment, whether the agent published a review, posted no verdict, or

- failed mid-run (D6). Re-runs edit the same comment in place via the existing

- sticky marker — they do not append a second one.

-

- The comment and the published review body both render from

- `render_deterministic_review_block()` in `findings/ledger.py` (D7), so the two

- surfaces cannot drift. The agent cannot suppress the block by supplying its own

- copy of the markers — dedupe keeps the server's version.

-

- The block always contains, in order:

-

- 1. **Run header** — outcome, verdict diagnostic, decision verdict and reason,

- model actually used, attempt count, token summary, run URL, reviewed SHA,

- publication path, and whether 422 recovery demoted inline comments into the body.

- 2. **Pre-merge checks** — analyzers (dispatched lenses or packet summary),

- static checks from `deterministic_checks`, CI intelligence pointer, trust

- tier.

- 3. **Change-scoped findings** — typed packet rows (`scope="change"`); `_No

- change-scoped findings recorded._` when empty.

- 4. **Run health** — collapsed `<details>` with `scope="run"` findings when

- present; omitted when none.

- 5. **Agent summary or rejection** — when a verdict exists, the agent's summary

- quoted beneath the deterministic rows; when it does not, an explicit

- `No verdict recorded — reason: <typed rejection>` line.

-

- The same block is merged into the review body as a mandatory preamble through

- `merge_deterministic_preamble_into_review_body` in `mcp/review.py`, applied

- last so nothing can be appended above it.

-

- **Token summary band.** The **Tokens** row reports `used (target N, ceiling M)`,

- optionally `over target` when spend crossed `runBounds.tokenBudget` but remains

- below the ceiling, and `by phase: …` when call sites annotated `record_tokens(…,

- phase=…)`. `tokenBudget` is the soft target; `tokenBudgetTolerance` (default

- `0.10`) defines the hard ceiling as `target × (1 + tolerance)`. Tolerance `0`

- restores strict enforcement at the target.

-

- **`raised_by` on the record.** Terminal-submission finding rows and the

- published review body carry `raised_by` for multi-reviewer attribution (see §8).

- The merge-evidence packet's typed `Finding` model does **not** include

- `raised_by` — provenance lives on the terminal layer and the deterministic run

- record, not in packet `findings[]`.

-

- ## 9. Address-reviews checks

-

- When mergecraft is on the receiving end of review comments (`AddressReviews` mode), each thread is checked for:

-

- - Whether the request still stands against current code — a stale request gets a reply, not a change.

- - Whether the proposed fix would be bloat in context; if so it's reverted rather than committed.

- - Whether the diff contains only intended changes, with no debug artifacts left behind.

- - Whether reply and resolve happened together — both or neither, and never before the fix is live on the remote.

- - Whether a refuted finding was recorded as a withdrawn finding (group 7).

-

- ===== FILE: docs/ANALYZERS.md =====

-

- # Analyzer catalog

-

- Shipped mergeCraft catalog analyzers. Rows are generated from manifests — run ``uv run python -m mergecraft.analyzers.catalog_docs`` to refresh.

-

-

- > **Provider configuration (catalog slugs, credential detection, `mergecraft auth <provider>`) is documented in [Authentication](../docs/authentication.md).** This page is the *analyzer* catalog — the rows below are deterministic, manifest-driven tools (`actionlint`, `zizmor`, `ShellCheck`, `Hadolint`, …) the reviewer runs mechanically. The Nous Research / DeepSeek V4 Flash path (provider id `nous`, catalog slug `nous/deepseek/deepseek-v4-flash`) is a *provider*, not an analyzer, and lives in the README's [Authentication table](../docs/authentication.md) alongside Anthropic, OpenAI, Google, and Cursor. Set up its secret with [`mergecraft auth nous`](../docs/authentication.md); see [issue #57](https://github.com/alexhawat/mergeCraft/issues/57) for the rationale.

-

- | id | category | languages | default | runtime | trust | exclusive group | notes |

- |----|----------|-----------|---------|---------|-------|-----------------|-------|

- | `actionlint` | ci | — | auto | managed | untrusted | — | — |

- | `agentsec` | security | — | enabled | repo-native | untrusted | — | — |

- | `antislop` | quality | python, javascript, typescript | disabled | repo-native | trusted | — | — |

- | `ast-grep` | security | python, javascript, typescript, go, java, rust, c, cpp, yaml | auto | managed | untrusted | pattern-scanner | Substrate for a future native policy engine — not built in C3. |

- | `bandit` | security | python | auto | repo-native | trusted | — | — |

- | `basedpyright` | lint | python | auto | repo-native | trusted | python-typecheck | — |

- | `biome` | lint | javascript, typescript | auto | repo-native | trusted | js-lint | — |

- | `blinter` | lint | batch | auto | managed | trusted | — | requires non-Linux runner — Windows batch lint not supported on Linux (C6) |

- | `brakeman` | security | ruby | auto | repo-native | trusted | — | — |

- | `buf` | contract | — | auto | managed | untrusted | — | — |

- | `bundler-audit` | vuln | ruby | auto | repo-native | trusted | — | — |

- | `cargo-audit` | vuln | rust | auto | repo-native | trusted | — | — |

- | `cargo-deny` | license | rust | auto | repo-native | trusted | — | — |

- | `checkmake` | lint | make | auto | managed | trusted | — | — |

- | `checkov` | security | terraform, cloudformation | auto | managed | untrusted | — | manifest-only — checkov binary not bundled for linux-amd64 (provisioning gap; use repo-native install or wait for pinned provenance) |

- | `circleci` | lint | yaml | disabled | managed | untrusted | — | — |

- | `clang-tidy` | lint | c, cpp | disabled | container | trusted | — | requires compile_commands.json — mergeCraft never guesses compiler flags (C4) |

- | `clippy` | lint | rust | auto | repo-native | trusted | rust-lint | — |

- | `cppcheck` | lint | c, cpp | auto | managed | trusted | — | — |

- | `detekt` | lint | kotlin | auto | repo-native | trusted | — | — |

- | `dotenv-linter` | lint | — | disabled | managed | trusted | — | Values never printed in findings (D8). |

- | `ember-template-lint` | lint | ember | auto | repo-native | trusted | — | — |

- | `eslint` | lint | javascript, typescript | auto | repo-native | trusted | js-lint | — |

- | `flake8` | lint | python | disabled | repo-native | trusted | python-lint | Legacy opt-in — disabled by default; enable via config override. |

- | `fortitude` | lint | fortran | auto | managed | trusted | — | manifest-only — Fortitude not bundled on Linux runners (C6 out of scope) |

- | `golangci-lint` | lint | go | auto | managed | trusted | go-lint | — |

- | `govulncheck` | vuln | go | auto | repo-native | trusted | — | — |

- | `hadolint` | lint | docker | auto | managed | untrusted | — | — |

- | `htmlhint` | lint | html | auto | repo-native | trusted | — | — |

- | `infer` | security | java, c, cpp | disabled | container | trusted | — | requires compilation database and build — container-only heavyweight (C4) |

- | `jscpd` | quality | javascript, typescript, python | auto | repo-native | trusted | — | — |

- | `knip` | quality | javascript, typescript | auto | repo-native | trusted | — | — |

- | `languagetool` | lint | text | disabled | container | trusted | — | manifest-only — LanguageTool runtime not bundled on Linux runners (C6 out of scope) |

- | `luacheck` | lint | lua | auto | repo-native | trusted | — | — |

- | `markdownlint` | lint | markdown | auto | repo-native | trusted | — | — |

- | `mypy` | lint | python | auto | repo-native | trusted | python-typecheck | — |

- | `oasdiff` | contract | — | auto | managed | untrusted | — | — |

- | `opengrep` | security | python, javascript, typescript, go, java, ruby, rust, yaml | disabled | managed | untrusted | pattern-scanner | — |

- | `osv-scanner` | vuln | — | auto | managed | untrusted | dependency-vuln | — |

- | `oxlint` | lint | javascript, typescript | auto | repo-native | trusted | js-lint | — |

- | `phpcs` | lint | php | disabled | repo-native | trusted | php-lint | Legacy opt-in — disabled by default; phpstan is the default PHP signal. |

- | `phpmd` | lint | php | disabled | repo-native | trusted | — | Legacy opt-in — disabled by default; phpstan is the default PHP signal. |

- | `phpstan` | lint | php | auto | repo-native | trusted | — | No phpstan.neon/neon.dist → runs at --level=0 (D12). |

- | `pmd` | lint | java | auto | managed | trusted | — | — |

- | `presidio` | security | — | disabled | container | trusted | — | Container-only; high-confidence entity types only. |

- | `prisma-lint` | lint | prisma | auto | repo-native | trusted | — | — |

- | `psscriptanalyzer` | lint | powershell | auto | managed | trusted | — | requires non-Linux runner — Windows/macOS only (C6 out of scope) |

- | `pylint` | lint | python | disabled | managed | trusted | python-lint | Legacy opt-in — disabled by default; enable via config override. |

- | `pyright` | lint | python | auto | repo-native | trusted | python-typecheck | — |

- | `regal` | lint | rego | auto | managed | trusted | — | — |

- | `rubocop` | lint | ruby | auto | repo-native | trusted | ruby-lint | — |

- | `ruff` | lint | python | auto | repo-native | trusted | python-lint | — |

- | `semgrep` | security | python, javascript, typescript, go, java, ruby, rust, yaml | enabled | managed | untrusted | pattern-scanner | — |

- | `shellcheck` | lint | shell | auto | managed | untrusted | — | — |

- | `shopify-theme-check` | lint | liquid | auto | repo-native | trusted | — | manifest-only — Shopify Theme Check not bundled on Linux runners (C6 out of scope) |

- | `smarty-lint` | lint | smarty | auto | repo-native | trusted | — | manifest-only — Smarty Lint not bundled on Linux runners (C6 out of scope) *.tpl extension is ambiguous (Go templates, Terraform, Smarty) — enable only when .smarty-lint.json confirms Smarty intent. |

- | `sqlfluff` | lint | sql | auto | managed | trusted | — | Dialect is mandatory — skip when repo declares none. |

- | `squawk` | migration | — | auto | managed | untrusted | — | — |

- | `stylelint` | lint | css | auto | repo-native | trusted | — | — |

- | `swiftlint` | lint | swift | auto | managed | trusted | — | requires non-Linux runner — SwiftLint needs macOS (C6 out of scope) |

- | `tflint` | lint | terraform | auto | managed | untrusted | — | — |

- | `trivy` | vuln | — | auto | managed | untrusted | dependency-vuln | — |

- | `trufflehog` | secrets | — | auto | managed | untrusted | — | verify off by default; impossible on fork PRs (C2). |

- | `tsc` | lint | typescript | auto | repo-native | trusted | — | — |

- | `typos` | lint | python, javascript, typescript, rust, go, markdown | auto | repo-native | trusted | — | — |

- | `vulture` | quality | python | auto | repo-native | trusted | — | — |

- | `yamllint` | lint | yaml | auto | managed | untrusted | — | manifest-only — yamllint binary not bundled for linux-amd64 (provisioning gap; use repo-native install or wait for pinned provenance) |

- | `zizmor` | ci | — | auto | managed | untrusted | — | — |

-

- ## Runtime x shell x trust x mode

-

- Which analyzers run is decided on four independent axes. Each can skip a

- manifest with a named reason — a skip is an outcome, never a failure, and it

- appears as an `unavailable` row in the Analyzers pre-merge summary.

-

- - **shell** (`shell:` in the workflow) — may mergeCraft execute anything the

- PR could have written? Enforced by `evaluate_manifest_for_shell()`.

- - **trust** (derived from the event) — `pull_request_target` and fork-head PRs

- are `untrusted`. Enforced by `evaluate_manifest_for_tier()`.

- - **egress** (declared ``network_allowlist``) — on untrusted events

- (`pull_request_target` or a fork-head PR), an analyzer that declares a

- non-empty allowlist is skipped with a named *egress policy* reason rather

- than granted host networking or left to fail as `unavailable`. Trusted

- events (`workflow_dispatch`, same-repo `pull_request`) keep today's

- behaviour: a declared allowlist drops network isolation so

- `osv-scanner`, `trivy`, `govulncheck`, and `bundler-audit` can reach

- their upstreams. On runners where `ip netns`, veth, and iptables are

- available, an operator may opt into the host FORWARD backend only in

- a disposable credential-free isolated runtime by setting

- `MERGECRAFT_FILTERED_EGRESS_ISOLATED_RUNTIME=1` in its parent environment.

- It is not a repo config field or an Action input. The GitHub Action

- image typically lacks `CAP_NET_ADMIN` / `CAP_SYS_ADMIN` for that host

- path; it instead uses a userspace backend (user+net namespace plus a

- parent TCP relay) so untrusted allowlisted analyzers can reach declared

- hosts and cannot reach undeclared ones without those capabilities.

- The host-FORWARD opt-in asserts external filesystem isolation; the

- network backend does not provide host filesystem confidentiality.

- Within either backend, untrusted allowlisted analyzers are confined to

- the *IP addresses* the declared hosts resolved to at setup time

- (sandbox enforcement, not `HTTP_PROXY`) — not the hostnames themselves.

- Enforcement is by IP, so a CDN-hosted allowlist entry admits every

- other domain that shares its IP, and setup-time resolution can drift

- from what the analyzer resolves inside the netns for a load-balanced

- upstream. A loopback CONNECT proxy in

- `tests/analyzers/support_allowlist_proxy.py` encodes the same hostname

- allowlist for unit tests. Enforced by

- `evaluate_analyzer_egress_policy()`, `filtered_egress_available()`,

- and `build_analyzer_sandbox_argv_for_run()`.

- Analyzer capabilities are dropped after mount setup, preventing

- namespace switching or enabling IPv6 again. The isolated runtime must

- use direct `unshare`; sudo-based backends are refused because sudo

- cannot safely carry the private payload-environment descriptor.

- Namespace helpers receive a minimal trusted environment. Payload

- environment values travel through a private descriptor and are

- restored only after capabilities are dropped, never in process argv.

- Analyzer timeouts kill the namespace init process and its descendants.

- The runtime must provision tools and libraries readable without DAC override; private

- installs owned by another user are intentionally inaccessible.

- Host INPUT destinations

- are always denied, IPv6 is disabled, and

- allowed IPv4 destinations admit every port/protocol. DNS permits

- arbitrary query names to configured external IPv4 resolvers (TCP/UDP

- port 53); DNS tunneling and shared-IP domains remain limitations.

- Loopback-only host resolvers cannot serve a namespace; configure an

- external resolver explicitly. Sessions require operator-enabled IPv4

- forwarding and never change the host's forwarding setting. Cleanup

- only removes owned state; forced termination may require operator

- cleanup of named resources, rather than a sweep of other sessions.

- - **mode** (`analyzers:` in the workflow) — `off | auto | full |

- untrusted-only`. Enforced by `evaluate_manifest_for_mode()` plus

- `resolve_selection_tier()`.

-

- On macOS no native analyzer isolation backend exists. Linux capability

- probes are skipped, and untrusted analyzers are refused before launch.

- Trusted local shell execution is an explicit execution permission, not

- an isolation guarantee; incomplete isolation produces a warning.

- Use `MERGECRAFT_DISPOSABLE_LINUX=1 make test-filtered-egress` as root

- only on a disposable Linux runner to exercise the real kernel boundary.

- This test cannot prove the standard Docker Action supports that boundary.

-

- Under `shell: disabled`, eligible runtimes are `managed` and `container`.

- Their argv is copied verbatim out of a manifest mergeCraft ships, and a binary

- the repo provides may not stand in for the pinned one, so nothing the PR

- authored is executed. `runtime: repo-native` stays withheld because it exists

- to run the *repo's* tool against the *repo's* config.

-

- | runtime | trust | `shell: disabled` | `shell: restricted` / `enabled` |

- |---------|-------|-------------------|----------------------------------|

- | `repo-native` (34) | `trusted` | withheld — `runtime` needs repo-provided tooling | runs on trusted events; skipped on untrusted |

- | `repo-native` (1) | `untrusted` | withheld — `runtime` needs repo-provided tooling | runs |

- | `managed` (12) | `trusted` | runs on trusted events; skipped with a reason on untrusted ones | runs on trusted events; skipped on untrusted |

- | `managed` (17) | `untrusted` | **runs** (pinned binary only) | runs |

- | `container` (4) | `trusted` | runs on trusted events; skipped with a reason on untrusted ones | runs on trusted events; skipped on untrusted |

-

- One documented exception to the runtime row: `agentsec`, `antislop`. They declare

- `runtime: repo-native` but `resolve_analyzer()` special-cases them before the

- repo-binary preference is consulted and `run_adapter()` executes them

- in-process — no subprocess, no argv, nothing the PR authored is run. The

- runtime axis asks whether PR content could steer what executes; for these

- the answer is no, so they stay eligible (#38).

-

- `antislop` is `trust: trusted` and `default_enabled: false`. Enabling it via

- `analyzers.overrides` on untrusted fork runs still no-ops when the trust axis

- skips trusted-only analyzers — opt in only on trusted events.

-

- ### The `analyzers:` mode axis

-

- `untrusted-only` runs only analyzers that need no secrets, no network and no

- PR-authored command construction: manifest selection is evaluated at the

- `untrusted` tier *and* the repo-tooling gate applies whatever the shell is.

- On `pull_request_target` and fork-head pull requests, `auto` resolves to it

- — a narrowing default, so a hardened workflow gets mechanical signal without

- loosening `shell:`. An unrecognised `analyzers:` value resolves there too,

- with a warning, rather than silently widening to `auto`.

-

- `full` requests more provisioning; it is never a trust override, and cannot

- re-admit a manifest the tier axis skipped.

-

- Counts below are analyzers passing selection, out of 68 shipped, with

- `shell: restricted` (the shell axis inert) so the mode axis is isolated.

-

- | mode | trusted event | untrusted event (`pull_request_target`, fork) |

- |------|---------------|-----------------------------------------------|

- | **`off`** | surface not registered | surface not registered |

- | **`auto`** | 68 of 68 | 18 of 68 — `auto` ⇒ `untrusted-only` |

- | **`full`** | 68 of 68 | 18 of 68 |

- | **`untrusted-only`** | 18 of 68 | 18 of 68 |

-

- Passing these axes is necessary, not sufficient: a `container` manifest

- is eligible but still reports `unavailable` wherever no container runtime is

- present, and the seven `declared_unavailable` manifests keep their own skip

- reason. In the shipped Action image that leaves the `managed` rows as the

- analyzers a `shell: disabled` run actually executes.

-

- Repo-declared `staticChecks` are a third thing and are **always** withheld

- under `shell: disabled`, on every event: they run command strings the PR

- author controls. They report `declared-but-cannot-run` rather than vanishing.

- No `analyzers:` value re-enables them.

-

- ## Overrides

-

- Enable or disable tools in ``.mergecraft/config.yaml``:

-

- ```yaml

- analyzers:

- overrides:

- golangci-lint:

- enabled: true

- ```

-

- See [CONTRIBUTING-ANALYZERS.md](CONTRIBUTING-ANALYZERS.md) to add a tool.

-

- ## Noise budget (D14)

-

- Inline review comments from analyzers and the reviewing agent share a single cap of **8** slots (W0.2 measurement; configurable via `analyzers.inlineBudget`). Placement is deterministic:

-

- - **Inline** — highest-priority findings up to the cap. Agent findings win tie-breaks over analyzer findings at the same severity and path.

- - **Mechanical overflow** — `source: analyzer` / `source: ci` findings that did not earn an inline slot render as a compact `### 🔧 Mechanical findings` table (tool, rule id, path:line). mergeCraft appends this section server-side at publish time.

- - **Deferred overflow** — `source: agent` findings that did not earn an inline slot render in `### 🗂 Deferred findings` with severity, path, line, and the **full finding body**. This lane is non-blocking (no inline anchor) and is also server-appended at publish time so overflow reasoning is never discarded.

- - **Nitpicks** — `Trivial` severity or `Low value` effort never occupy inline or deferred slots; they belong in the Nitpicks section.

-

- ## Verification gate (D11)

-

- `Critical` and `Major` findings are hypotheses until the read-only `mergecraft-verifier` subagent reads the cited code. That gate applies to analyzer, CI, and agent-authored findings.

-

- Verification depth is **independent** of inline placement (`analyzers.inlineBudget`, default 8). The verifier dispatch cap is `review.verificationBudget` (default **24**). Set it to **`0`** to verify every eligible finding with no cap. Three filters run in order: severity (skip `Minor`/`Trivial`), withdrawn memory (skip fingerprints already refuted under `## Withdrawn review findings`), then the verification budget — so pre-budget skips never consume slots. Over-budget fingerprints surface in `skippedOverBudget` for the open-PR ledger (W3).

-

- ## Execution preference

-

- For any given gate, in order — the first that can produce a verdict wins:

-

- 1. **`repo-native`** — the repo's own pinned toolchain, when this environment can run it.

- 2. **An existing CI result** — a check run the repo *declared* as proof of that gate (#36).

- 3. **A managed pinned binary**, then **a container**.

- 4. **Skip, with a named reason.** A skip is never a finding.

-

- ## CI evidence (#36)

-

- The Action image usually lacks `make`, the repo's venv, and its pinned toolchains, so a repo-native gate reports `unavailable` even when the consumer's own CI just proved the same thing. Declaring the mapping lets that finished CI stand in:

-

- ```yaml

- ciEvidence:

- gates:

- # <mergeCraft gate name>: <exact GitHub check-run name>

- lint: Verify (drift gates)

- sarifArtifacts:

- - ruff-sarif

- ```

-

- - **Declared only.** mergeCraft never infers that a check run *named* `lint` proves the `lint` gate — a pull request can add a workflow with any name it likes. With no `ciEvidence` block nothing is read and no extra API call is made.

- - **Green only substitutes.** A declared check run that passed rewrites the gate row to `satisfied-by-ci`. A declared check run that *failed* leaves the row alone and is reported as a `source: ci` finding instead.

- - **CI SARIF ingest on complete wait.** Declared `sarifArtifacts` are downloaded after `wait-for-ci` reaches `state=complete` (green or red CI). mergeCraft lists workflow runs for the PR head SHA and fetches matching artifacts from those runs. Wait states `timeout`, `absent`, and `skipped` skip ingest; `workflow_dispatch` does not pre-ingest because the wait job is PR-only.

- - **Wait outputs required.** Forward `wait-for-ci` `state` and `failed_count` into the review job as `MERGECRAFT_CI_WAIT_STATE` / `MERGECRAFT_CI_FAILED_COUNT` (or the `CI_STATE` / `CI_FAILED_COUNT` aliases). Without `state=complete` ingest is skipped.

- - **`actions: read` required.** The review workflow job must grant `actions: read` so artifact download succeeds. Without it GitHub answers 403 and SARIF ingest is skipped (the review still completes).

- - **CI SARIF catalog.** Tools that arrive via CI SARIF when declared in `ciEvidence.sarifArtifacts` and uploaded from the consumer's `ci.yml` include **ruff**, **mypy**, **bandit**, **actionlint**, **zizmor**, **semgrep**, and **trufflehog** — artifact names follow the `<tool>-sarif` pattern and must match both config and workflow upload steps. **trufflehog**'s catalog parser is JSONL; CI converts that output to SARIF so ingest stays one path. A clean scan still uploads a valid empty-results SARIF with tool metadata, never a 0-byte file.

- - **Reported, not blamed.** Bare check-run findings start non-blocking; SARIF `error` from declared `sarifArtifacts` keeps Major/Critical. `introduced_by_pr` stays `unknown` until `ci/blame.py` / `ci/flaky.py` attribute a finding to this PR.

- - **Redacted.** Log excerpts are truncated and passed through `analyzers/redact.py` before they enter a finding.

-

- ## SARIF upload to code scanning (#39)

-

- Opt-in, off by default. When enabled, mergeCraft exports the analyzer findings of a pull-request run as SARIF 2.1.0 and uploads them to GitHub code scanning, so mechanical findings stay readable when the review narrative is thin or when findings overflowed the inline comment budget.

-

- ```yaml

- # .github/workflows/mergecraft.yml

- permissions:

- contents: read

- pull-requests: write

- # Required for the upload. Without it GitHub answers 403 and mergeCraft

- # logs a warning — the review still completes.

- security-events: write

-

- jobs:

- review:

- steps:

- - uses: alexhawat/mergeCraft@<sha>

- with:

- sarif_upload: enabled

- ```

-

- Or in `.mergecraft/config.yaml` (the action input wins when it is set):

-

- ```yaml

- analyzers:

- sarifUpload: true

- ```

-

- What is and is not uploaded:

-

- - **Catalog analyzers only.** Only `source: analyzer` findings are eligible. `source: ci` findings carry truncated pipeline log excerpts and `source: agent` findings carry narrative; neither is uploaded, and raw logs never leave the process (D13).

- - **The clustered, placed set.** The upload reuses the findings the pipeline already clustered and placed, not the raw analyzer output, so cross-tool duplicates arrive as one alert (D14). It is *not* truncated at the inline comment budget — the overflow is exactly what this surface exists to show.

- - **Trust-gated.** Each finding's analyzer must still pass this run's `trust` x `shell` x `analyzers:` selection chain — the same predicates the pipeline calls, re-evaluated at upload time. A finding from a tool with no catalog manifest cannot be gated, so it is refused.

- - **Redacted before serialization.** `message`, `evidence`, `remediation` and `autofix` pass through `analyzers/redact.py` while still typed `Finding`s, before SARIF is built. `path` is left intact: it becomes `artifactLocation.uri`, and mangling it would detach the alert from its file.

- - **Never a gate.** A rejected upload — missing permission, code scanning unavailable, transport error — is logged at `warning` and the run continues.

-

- Check-run annotations are the documented alternative surface and are not implemented: they need `checks: write` instead, cap at 50 annotations per request, and largely repeat the inline review comments mergeCraft already posts.

-

- ===== FILE: docs/REVIEW-DOCTRINE.md =====

-

- # Review doctrine

-

- > **Agent-facing artifact:** `.github/skills/code-review/` is what mergeCraft injects

- > into the reviewing agent at runtime; this file is the human-facing rationale the

- > skill distills for prompts.

-

- Review-check reasoning from `review_checks.py`, `review_taxonomy.py`,

- `mcp/static_checks.py`, and `REVIEW-CHECKS.md`. W2 and W5

- build on these decisions — they are not recoverable from code alone.

-

- ## Mechanical gates vs findings

-

- **`unavailable` is not `failed`.** A gate whose executable is missing (no `make`, no

- linter on PATH) says nothing about the diff. Reporting it as a failure invents a finding.

- Only a real non-zero exit from an executable gate is evidence. The Action image ships

- `git`, `gh`, `jq`, `node`, and `npm` — not `make` — so every Makefile-discovered target

- lands as `unavailable` there unless the repo declares explicit `staticChecks` with binaries

- that exist in the image.

-

- ## Finding scope: change vs run (plan 12, D1–D2)

-

- Every `Finding` carries a `scope` axis: `change` or `run`. The default is `change`, so

- existing producers keep their meaning without silent reclassification.

-

- - **`change`** — a claim about the diff under review. These are the findings that can

- block merge after causality policy and severity grading.

- - **`run`** — a claim about how the review executed (trajectory auditor rows, environment

- observations, and similar process evidence). These are **advisory only** and can never

- block (D2). Run health is reported — in the evidence packet's `run_health` section, the

- deterministic run record, the `mergecraft` completion check summary, and the job step

- summary — never enforced as a second merge gate (D14).

-

- `blocking_findings()` in `agents/gates.py` is the single predicate: drop `scope == "run"`,

- apply `apply_causality_policy`, then test `BLOCKING_SEVERITIES`. `_has_blocker`,

- `_packet_has_blockers`, `decide_approval`, and `mcp/verdict.py::_blocks_approve` all route

- through it — two predicates that disagree is a defect class, not a style question (D3).

-

- Trajectory checks stamp `source="trajectory"`, `scope="run"`, and `introduced_by_pr="false"`

- (D4). All three fields matter: `introduced_by_pr` drives causality, `scope` drives gating,

- `source` drives rendering.

-

- ## Makefile discovery, not tool inference

-

- **`DISCOVERABLE_TARGETS` discovers Makefile targets, not tools.** The tuple

- `("lint", "format-check", "typecheck", "ci-static")` is offered in order when no

- `staticChecks` are declared. Nothing is inferred from file extensions; no interpreter or

- linter is substituted. The repo's own gate is the only gate.

-

- ## Never substitute a toolchain version

-

- When the repo has a tool, mergeCraft runs **the repo's copy** at **the repo's config and

- version**. A reviewer carrying its own interpreter manufactures findings: `except A, B:`

- is a `SyntaxError` under Python 3.13 and legal under 3.14 (PEP 758), which this project

- requires. The module docstring, tool description, and mode prompt all encode this rule.

-

- ## Finding fingerprints

-

- **`finding_fingerprint()` = `sha256(path + "\n" + casefolded whitespace-collapsed body)[:24]`.**

- Whitespace and case are normalized so re-wrapping a comment does not change the hash, letting

- a later run recognize a finding it already raised. The marker is stamped server-side in

- `mcp/review.py`. **Cost:** paraphrases and minor rewordings produce new fingerprints; the

- tradeoff favors stable dedup over semantic similarity.

-

- ## Output cap

-

- **`MAX_OUTPUT_CHARS = 8_000`** caps combined stdout+stderr embedded in prompts. Raw tool output

- beyond this truncates — a design constraint for W4's move to file-based parsing. Mechanical

- gate output is evidence, not the finding itself.

-

- ## Subagent deny-list

-

- **`subagent_denied_tool_names()` derives from every MCP tool with `mutates=True`.** If that

- list is empty, startup **raises** — refusing to run a review subagent with the mutation gate

- effectively disabled. The verification agent (W7) inherits the same guard.

-

- ## Verification covers every source, including ourselves (C6)

-

- **A `Critical`/`Major` finding is a hypothesis until a second read-only agent has read the

- cited code — whatever wrote it.** `should_verify()` was always severity-only; the source

- condition lived in its two call sites (`analyzers/review_gate.py`, `ci/verification.py`), both

- of which only ever fed it tool output. The effect was that the noisiest source — the reviewing

- model's own findings — was the one source that never got checked. `verify_agent_findings` and

- `record_finding_verdict` close that, on the same terms as the analyzer path: severity gate,

- withdrawn-memory skip, and a dispatch cap.

-

- **The cap is the inline budget, not a new knob.** Verification exists to protect what gets

- published, so it can never cost more than publication does: dispatches are capped at

- `analyzers.inlineBudget` and spent on `Critical` before `Major`. **Cost:** on a diff with more

- than `inlineBudget` blocking findings, the overflow publishes unverified — the alternative

- (unbounded judge dispatches on the worst diffs) is worse.

-

- **A `drop` is durable.** It writes the verifier's reason under `WITHDRAWN_FINDINGS_HEADING` with

- the finding's own fingerprint, so the same claim is skipped before verification on every later

- run — the same section, parser and identity analyzer suppression already uses. Verifying a

- finding the author refuted last month is the failure this prevents.

-

- ## Recall pass — the verifier's mirror (RC10, D1, D7)

-

- **Every stage before publication subtracts findings; the recall pass is the one stage that may

- add them back.** After aggregation the orchestrator dispatches `mergecraft-recall` with the

- authoritative diff and the draft finding list. The subagent is read-only, uses the same

- subagent deny-list as `mergecraft-reviewer`, and may return only findings absent from that

- draft — paraphrases and overlaps are filtered through `findings.dedup.dedupe_findings`, not a

- second matcher.

-

- **Output is always deferred (D1).** Recall findings publish in `### 🗂 Deferred findings`

- regardless of the severity the subagent claims. They never consume inline budget and never

- block merge. A precision gain bought by silently dropping recall output would be a regression;

- the novelty filter and deferred placement are the paired constraint.

-

- **Default off, dogfooded here (D7).** `review.recallPass` defaults `false` for consumers — it

- costs a subagent dispatch per review. mergeCraft's own `.mergecraft/config.yaml` keeps it off

- until the recallPass schema lands on `pre-0.0.1` (the PR-branch action pin cannot consume the

- field before merge); dogfood re-enables it after that merge.

- The W7 corpus gate (`evaluate_recall_pass_corpus`) must show first-pass recall up with the DG1

- precision corpus flat or better before the wave ships.

-

- ## LLM judges are secondary evaluators (D14, #45)

-

- **The verifier is an LLM judging an LLM, so it is pinned, logged, ordered last, and never

- decisive alone.**

-

- - **Ordered last.** `verify_agent_findings` returns `ready:false` and `record_finding_verdict`

- refuses a verdict until `run_analyzers` or `run_static_checks` has run. Deterministically

- checkable facts are settled by tools; the judge only rules on what tools cannot decide.

- - **Pinned.** `PINNED_JUDGE_MODELS` fixes the judge model per provider (`claude` →

- `claude-sonnet-5`) and `agents/claude.py` dispatches from that same constant, so the model

- recorded and the model run cannot diverge. A provider without a pin still records a complete

- identity, marked `model_pinned=false`.

- - **Logged.** Every verdict carries judge provider, model, whether the model was pinned,

- `VERIFIER_JUDGE_VERSION` and `VERIFIER_RUBRIC_VERSION`. A rubric edit bumps the version rather

- than silently reinterpreting archived verdicts.

- - **Outcome-based.** The rubric is five binary questions about the code (`cited-code-exists`,

- `mechanism-holds`, `reachable`, `introduced-here`, `not-already-refuted`). Nothing in it scores

- quality, style, tone, or length — a judge that grades prose grades noise.

- - **Not decisive on high-stakes lanes.** On the `high` blast-radius lane a `drop` is escalated for

- a second judge or a human instead of being written to the withdrawn section. Retracting a real

- finding on a migration or an auth change is the expensive direction to be wrong in.

-

- **Cost:** a run whose reviewer never calls the deterministic tools gets no verification at all.

- That is deliberate — a judge with nothing to be secondary to is the failure mode #45 names.

-

- ## Shell permission and static checks

-

- **`run_static_checks` is withheld under `shell: disabled`.** Gates execute commands the repo

- config names; on a pull request those are commands the PR author controls. Offline

- `mergecraft review` keeps the tool because config and tree belong to the operator.

-

- ## Provenance

-

- Recorded from the review-triage and mechanical-gates work (PR #20).

-

- ## Green is evidence, not proof (#41, W2)

-

- A passing check-run is **evidence**, never a **proof**. The merge decision

- is a function of durable, structured evidence — never of an agent's

- self-report. Two consequences follow:

-

- - **The agent's `approved` boolean is recorded but never sufficient.** It

- lives on the merge-evidence packet as `self_assessment` (with the

- reviewed `sha`) — a sibling of `decision`, not a substitute. When the

- agent's `self_assessment.approved == True` is the *only* positive

- signal, the verdict is `neutral` (or the packet's explicit `decision`

- if set upstream), not `auto_merge`. That is the #41 hard rule; it is

- pinned by `tests/evidence/test_self_assessment.py::test_self_assessment_alone_blocks_auto_merge`.

- - **The decision is monotone in blockers.** Any `Critical` or `Major`

- finding yields `failure` regardless of the agent's `approved` value;

- `run_succeeded == False` yields `neutral`; `tier == "untrusted"`

- yields `neutral`. A "green" check-run never outvotes a blocker; an

- agent's `approved=True` never outvotes a blocker. See

- `mergecraft.agents.gates.decide_approval` (the security-trust-boundary

- plan's Batch D contract, D5) and `tests/status_checks/test_decide_approval.py`.

-

- ### Run outcome taxonomy (D3, W5)

-

- A run ends in exactly one of six named outcomes —

- `mergecraft.run_outcome.RunOutcome`: `passed`, `failed`, `inconclusive`,

- `infra_error`, `timed_out`, `configuration_error`. `MainResult.outcome`

- carries it, and two mappings key off it:

-

- | Mapping | Lives in | Notes |

- |---------|----------|-------|

- | outcome → `mergecraft` completion check-run conclusion | `mergecraft.run_outcome.RUN_OUTCOME_CONCLUSION` | Only `passed` → `success`; `failed` → `failure`; `timed_out` → GitHub's literal `timed_out`; everything else → `neutral`. |

- | outcome → `result` output JSON | `cli/gha_cmd.py` (`_structured_failure_result`, W5.3) | `{"outcome": ..., "error": {"code": "mergecraft.<outcome>", "message": <redacted>}}` on any failure path — not just `::error::` + exit 1. |

-

- The `mergecraft-approval` check stays governed by the existing

- `decide_approval` 3-way conclusion above: every outcome except `passed`

- feeds it `run_succeeded=False` (`run_outcome.run_succeeded_for_outcome`), so

- this taxonomy is additive detail on top of the pre-W5 boolean, never a

- looser gate.

-

- ### Evidence weighting

-

- What the merge-evidence packet carries, and how each signal weights:

-

- | Signal | Weight | Notes |

- |--------|--------|-------|

- | `findings: list[Finding]` (typed, taxonomy-validated, `extra="forbid"`) | **structural** | One `Critical` or `Major` finding blocks. Source/severity/confidence preserved verbatim. |

- | `deterministic_checks: list[DeterministicCheck]` (name, status, command) | **mechanical** | Only `passed` / `failed` count as positive / negative evidence; `unavailable` / `declared-but-cannot-run` / `timed_out` are honest skips — not silent passes (PR #17 vocabulary, see `REVIEW-CHECKS.md`). |

- | `ci_check_runs` / `ci_intelligence.annotations` | **mechanical** | Per-ref check-suite outcome + log-cluster signatures, flaky vs stable, blame-attributed. |

- | `self_assessment.approved: bool` + `self_assessment.sha` | **advisory only** | Recorded; never the sole positive input. The packet's `decision` row is authoritative. |

- | `decision: { verdict, reason, decided_by }` | **authoritative** | Populated by `decide_approval`. When present on the packet, it wins over every other signal. |

-

- What is **never** an input to the verdict: the agent's prose narrative,

- the PR title / body / comment text (fenced), `result.output`, the model

- slag in tool output, or any other string that was not produced by a

- deterministic check on the diff. A behaviour-verification report is

- evidence about a running app, not a substitute for diff review and not a

- typed `Finding`. `skipped` or `blocked` is not a pass: blocked names the

- missing input; skipped means the run did not execute. The report is

- fenced before it reaches any prompt. The merge-evidence packet is the

- single artifact a human or a later tool reads to reconstruct why a PR

- was auto-merged, blocked, or escalated; it is durable, versioned, and

- the schema is derived from the Pydantic models (`mergecraft.evidence.

- packet.PACKET_SCHEMA_VERSION`, D7).

-

- ### Honesty about unavailable signals (W2.4)

-

- Where a signal source is unreachable, the packet records it as

- `unavailable` with a reason — never silently as "passing". This is the

- same honesty rule PR #17 landed for `staticChecks`: an environment

- without `make`, a missing linter binary, an unreachable check-suite API,

- or a CI provider mergeCraft cannot reach — all surface explicitly. The

- "green" in a check-run summary is the verdict's input; "no signal" is a

- verdict's input that says *the review has nothing to attest to*.

-

- ## Rejected: numeric confidence and a flattened finding schema (C12, C22)

-

- Both of these have been proposed, evaluated, and **rejected**. They are

- recorded here because they are the kind of proposal that returns — each

- looks like a simplification and each would remove a load-bearing

- mechanism.

-

- ### Confidence stays categorical

-

- `FINDING_CONFIDENCES` is `certain` / `likely` / `possible`

- (`review_taxonomy.py`), and it stays that way. A numeric

- `confidence: 0.94` is an **uncalibrated model self-report**: nothing

- measures it, nothing validates it, and no two runs mean the same thing by

- it. Its real cost is what it invites — a `min_confidence: 0.8` filter,

- which is a threshold on a meaningless number, presented to operators as

- if it were a dial with units.

-

- This is the same principle as *Green is evidence, not proof* above,

- applied one level down: the merge decision does not consume an agent's

- self-report, and neither should finding triage. Depth comes from

- **evidence** — the cited code, the verifier's verdict, the deterministic

- checks that ran — not from a number the model chose.

-

- Consequently, `review.min_confidence` will not be added to `RepoSettings`

- even if the rest of a noise-control block is (C17).

-

- ### The `Finding` schema is extended, never flattened

-

- A flatter finding shape has been proposed:

- `path`→`file`, `message`→`title`, `remediation`→`suggestion`,

- `autofix`→`patch`, dropping `fingerprint`, `introduced_by_pr`,

- `cluster_id`, `tool`, and `rule_id`.

-

- Every dropped field is load-bearing, and dropping them breaks features

- the same proposal asks for elsewhere:

-

- | Field | What depends on it |

- |---|---|

- | `fingerprint` | Incremental re-review dedup, the withdrawn-findings memory, thread resolution |

- | `introduced_by_pr` | Scope filtering (`analyzers/scope.py`) — the difference between "this PR broke it" and "it was already broken" |

- | `cluster_id` | CI log clustering |

- | `tool` / `rule_id` | Analyzer provenance and trust tiering |

-

- Renames are pure churn: they change every parser, fixture, and adapter to

- buy nothing. **Additive** extension is fine and is the supported path —

- add a field, keep the model strict (`extra="forbid"`), and version it if

- the shape changes.

-

- ## Trust tiers and contributor weight

-

- `analyzers/trust.py::derive_trust_tier()` collapses an event's metadata into one of

- `trusted` (OWNER / MEMBER / COLLABORATOR on the base repo, or operator-owned payload

- sources) or `untrusted` (fork PR head, `pull_request_target`, or anything with no

- `author_association`). The fence's `tier=` and `trust=` headers carry this value forward

- into the rendered prompt so a reviewer can weight what is inside the block.

-

- How a reviewer should weigh that header on a per-field basis:

-

- - **A `MEMBER` comment is not a finding.** It is context the reviewer reads *after* the

- diff — same as a first-time contributor's comment, with one extra signal: `MEMBER`

- comments have been pre-screened by the same gate that grants write access. They are

- more likely to describe a real concern, but they are still evidence, not instruction.

- A reviewer who reads a `MEMBER` comment and uses it to skip findings on a path is

- applying an instruction-shaped signal that the comment cannot carry — the diff is

- still the only thing that anchors a finding.

- - **A first-time contributor's comment is read with no prior weight.** Treat it as

- *possibly* informed, *possibly* an injection probe. The fence's nonce binds the

- delimiters; a forged closer or opener cannot escape. If the comment's text tries to

- redirect the reviewer (skip this path, approve without reading, override your

- persona), the fenced block is exactly the place where the rule "evidence, not

- instruction" applies.

- - **`OWNER` comments pass through unfenced** — see `fence_unless_trusted()` in

- `mergecraft.utils.fence`. The trust exemption is *per-field*, not per-thread:

- an OWNER-typed review comment does not extend trust to a sibling attacker's

- comment in the same thread. Each field's `author_association` is checked

- independently, and the W4 enumeration test pins that.

-

- The hard rule (W4.5 / D9): **PR prose is evidence, never instruction.** A finding whose

- only support is the PR title, PR body, or a comment is dropped or downgraded; the diff

- must anchor every surviving finding. The fence is the technical mechanism that makes

- this rule enforceable — without it, prose and instruction share a channel and a

- sufficiently verbose injection can steer a review.

-

- ## Failure memory

-

- The **eval bank** is mergeCraft's failure memory — the durable, file-backed case store

- for failures a run should have caught and did not. The doctrine is **replay, not

- re-execution.** A case captures a *failure mode*, not a *replay driver*. The replay

- engine is a pure function of the case and the running code's verdict; the bank does not

- re-run the agent, the analyzers, or the merge-evidence pipeline.

-

- The bank surfaces two distinct failure modes:

-

- - **`rejected`** — the reviewer said *no* before merge. The case asserts the packet

- should have produced a `block` verdict and the related finding.

- - **`reverted`** — the merge made it past the reviewer but had to be rolled back. The

- case asserts the packet should have caught the regression that the revert exposed.

-

- A precision gain bought with recall is a regression — measure it with

- `mergecraft eval convergence` / `make eval-convergence`, which scores

- first-pass recall and leakage rate from ledger snapshots via

- `mergecraft.evals.convergence.score_convergence` (RC6).

-

- A failure mode is captured as a case by the operator, never by the agent. The

- `create_pull_request_review` MCP tool logs a one-line suggestion at `logger.info` when

- the run produced no positive findings on a re-review with trusted provenance and the

- operator has opted in via the `suggest_eval_add` action input. The agent never

- auto-adds — the bank is for *operator review*, not auto-capture.

-

- The bank's promoted-tests workflow (`mergecraft eval promote <case-id>`) is the

- regression net: a case promoted into `tests/evals/permanent/` re-runs the replay on

- every CI. Drift surfaces as a failing pytest assertion alongside the rest of the suite.

-

- The packet records the breadcrumb-and-summary of which bank cases the run attached to

- its verdict via `MergeEvidencePacket.evals` (a typed `list[EvalMetadata]`; schema

- `1.2.0`). The full case continues to live under `evals/cases/<case_id>.md`; the packet

- row is the operator-facing reference.

-

- The cross-reference is **one-way**: the doctrine refers to the bank, the bank refers

- back to the doctrine. The bank does not embed doctrine; the doctrine does not embed

- case payloads. The protocol is the join key (`case_id`).

-

- ## Terminal submission requirement (VP2)

-

- mergeCraft separates three layers that older review flows conflated:

-

- 1. **Terminal submission** — `submit_review_verdict` records the agent's

- `approve` / `request_changes` choice, summary, and structured findings on

- `ToolState`. One authoritative payload per attempt; identical re-submission

- is idempotent, a conflicting payload is rejected.

- 2. **Structural verdict** — `decide_approval(findings, *, run_succeeded, tier)`

- is a pure function of typed findings and run state. It never reads agent

- prose, `result.output`, or `ApprovalRecord.would_approve`.

- 3. **Publication** — `publish_pull_request_review` is the internal publisher

- (not an MCP tool). `create_pull_request_review` maps legacy params through

- `validate_submission`, derives the body from the terminal submission, and

- routes through the same publisher. Publication is idempotent per

- `(pull_number, commit_id)`; a second publish short-circuits. The published

- body must match the terminal submission — probe or health-check strings are

- hard failures when a submission is bound.

-

- **Per-run token band (`runBounds`).** `tokenBudget` is the soft target.

- `tokenBudgetTolerance` (default `0.10`) defines the hard ceiling as

- `target × (1 + tolerance)`. Crossing the target warns once and annotates the

- run record; crossing the ceiling raises `BudgetExhausted`. Tolerance `0`

- restores strict enforcement at the target. The run-record **Tokens** row and

- job step summary name target, ceiling, over-target status, and per-phase totals

- when `record_tokens(…, phase=…)` was used.

-

- **Enforcement default (VP4).** `gates.terminal_verdict` defaults to `enforce`.

- A provider success without a usable terminal submission maps to

- `RunOutcome.inconclusive`. Operators can set `gates.terminal_verdict: shadow` to

- keep diagnostics-only rollout.

-

- **Schema vs semantic validation.** Pydantic enforces the wire shape (closed

- verdict enum, `extra="forbid"`). `validate_submission` then applies semantic

- rules in trusted mergeCraft code: `request_changes` with zero findings,

- `approve` over a verifier-confirmed Critical/Major blocker, and `approve` with

- a failing required deterministic check are all rejected with a typed

- `rejection_reason`. A rejected submission leaves

- `terminal_submission_received=false` — fallback-eligible, same as no

- submission (D8). The approve path fails closed on *unverified* blockers too: a

- Critical/Major analyzer finding that is neither verifier-confirmed nor withdrawn

- rejects `approve`, so skipping verification is not a route to approval.

-

- **Outcome resolution.** In `Review` and `IncrementalReview` modes, a provider

- that returns process success without a usable terminal submission maps to

- `RunOutcome.inconclusive` (`neutral` check conclusion), not `passed`. Build and

- other non-review modes are unchanged.

-

- **Fallback.** The semantic-fallback chain advances on

- `not terminal_submission_received`, not on verdict content. A valid

- `request_changes` with confirmed findings is a usable review result; a missing

- or semantically rejected submission is not.

-

- See also [`REVIEW-CHECKS.md`](../REVIEW-CHECKS.md#terminal-verdict-vs-structural-verdict-vp2).

-

- ## Provenance

-

- (See the heading above for sources.)

-

- ===== FILE: docs/TRACING.md =====

-

- # Tracing — configuration, sinks, redaction, retention

-

- > Status: **complete**. Batch A (W2) covers the config schema, canonical span

- > model, local JSONL sink, and redaction boundary; Batch B (W4) wires the

- > production span tree; Batch C (W6) ships the `stream-json` migration

- > for per-tool / per-LLM spans; Batch D (W8) ships remote exporters, the

- > optional extra, CLI / `action.yml` inputs, and complete documentation.

- > Reference issue: [#56][i56].

-

- [i56]: https://github.com/alexhawat/mergeCraft/issues/56

-

- ## Why a tracing block

-

- mergeCraft runs are short-lived and an exhausted runner leaves nothing

- behind. When a review is slow, wrong, or expensive, there is no record of

- which model served it, which tools the agent called, or what was passed in.

- This block writes a per-run span tree to local files (and, behind the

- optional `tracing` extra, to remote exporters) so an operator can answer

- "why was this blocked?".

-

- A repo that does not declare a `tracing:` block sees identical behaviour,

- identical performance, and zero egress — the default is off, and the

- disabled path is a true no-op (no directory is created).

-

- ## Quick start: enable Logfire tracing

-

- The shortest path from a fresh checkout to spans landing in Logfire:

-

- ```bash

- # 1. install the optional extra (one time)

- uv sync --extra tracing

-

- # 2. run the auth command (validates the token, writes .env + gh secret)

- mergecraft auth logfire

-

- # 3. verify wiring (token is redacted in the table)

- mergecraft config tracing

-

- # 4. ship a trace

- mergecraft review --tracing --tracing-to logfire

- ```

-

- `mergecraft auth logfire` accepts `--scope local|github|both` (default

- `both`). `local` writes `MERGECRAFT_LOGFIRE_TOKEN` + `MERGECRAFT_TRACING_PROJECT`

- into `.env`; `github` calls `gh secret set LOGFIRE_TOKEN` on the origin

- repo; `both` does both. The validator probes

- `GET https://logfire.pydantic.dev/api/v1/projects` (parity with the other

- `auth` providers) and rejects the token on `401`/`403` before any state

- changes. The `[tracing]` extra must be installed for spans to actually leave

- the runner — the command prints a warning when the extra is missing but does

- not auto-install (BYOK, convention 5).

-

- For the GitHub Action, the workflow step should pass

- `tracing-to: logfire` + `logfire-token: ${{ secrets.LOGFIRE_TOKEN }}` (the

- `LOGFIRE_TOKEN` secret is the same value `auth logfire --scope github`

- sets). The Action input `logfire-token` is exported to

- `MERGECRAFT_LOGFIRE_TOKEN` **before** sink initialisation

- (`export_tracing_env_from_action_inputs` in `action/inputs.py`) — an empty

- or absent input does not clobber an already-set env var. The project label

- is read from `MERGECRAFT_TRACING_PROJECT` or the YAML

- `tracing.sinks[].project` field.

-

- Rather than editing the workflow by hand, let the CLI write it:

-

- ```bash

- mergecraft tracing logfire wire-workflow --step all --region eu # dry-run diff

- mergecraft tracing logfire wire-workflow --step all --region eu --apply

- ```

-

- This inserts `tracing: "true"`, `tracing-to: logfire`, and

- `logfire-token: ${{ secrets.LOGFIRE_TOKEN }}` into the step's `with:`, plus

- `MERGECRAFT_TRACING_PROJECT` (and, with `--region`, `MERGECRAFT_TRACING_REGION`)

- into its `env:`. `unwire-workflow` strips all of them.

-

- ### Data region

-

- Logfire serves region-specific OTLP ingest hosts — `logfire-us.pydantic.dev`

- and `logfire-eu.pydantic.dev` — and the resolver **defaults to `us`**. A write

- token is regional: `pylf_v{N}_eu_…` belongs to the EU project and its spans are

- rejected by the US host. Set the region explicitly whenever the token is not a

- US one:

-

- | Surface | How |

- | ------- | --- |

- | Local | `mergecraft tracing logfire enable --region eu` (writes `MERGECRAFT_TRACING_REGION` to `.env`) |

- | Action | `mergecraft tracing logfire wire-workflow --region eu --apply` (writes it to the step's `env:`). The Action applies that env onto the logfire sink even when tracing is enabled via the `tracing` input rather than `MERGECRAFT_TRACING` |

-

- Both paths converge on the same precedence layer

- (`cli/tracing_precedence.py`) and the same resolver

- (`tracing/resolve.py`), so the local `.env` shape and the workflow `env:`

- shape carry identical meaning.

-

- ### The `[tracing]` extra in the Action image

-

- The Action runs from `Dockerfile`, which installs

- `uv sync --frozen --no-dev --extra tracing`. Without that extra the sink

- factory degrades a `logfire` / `otel` sink to `NullSink` with a warning

- (`src/mergecraft/tracing/sinks.py`) — the workflow looks correctly wired and

- exports nothing. If you fork the image build, keep the extra.

-

- ## Config schema

-

- ```yaml

- tracing:

- enabled: true # default: unset (treated as off); bool | null

- retentionDays: 30 # default: 30

- redaction: true # default: true

- content: redacted # default: redacted; off | metadata | redacted | full (OB2/D6)

- sinks:

- - type: jsonl_file

- path: .mergecraft/traces/

- # Batch D adds: logfire, otel (behind the [tracing] extra, D6).

- ```

-

- `enabled` is tri-state: `true`, `false`, or unset (`null`). Unset defers to

- the next precedence layer and is **not** the same as `false`. On the Action

- path the precedence is: `tracing` / `INPUT_TRACING` action input >

- `MERGECRAFT_TRACING` env > YAML `tracing.enabled` > default (unset → tracer

- off). See `src/mergecraft/action/inputs.py::apply_tracing_overrides`.

-

- ### Shorthand form

-

- The shorthand `to: local_files` is normalised into the canonical `sinks`

- list at parse time (D9) — exactly one shape exists downstream:

-

- ```yaml

- tracing:

- enabled: true

- to: local_files # expands to [{type: jsonl_file, path: .mergecraft/traces/}]

- ```

-

- ### Sink entry fields

-

- | Field | Type | Notes |

- | ------------- | ------------------- | ----------------------------------------------- |

- | `type` | string (required) | One of `jsonl_file`, `memory`, `logfire`, `otel`. |

- | `path` | string | Directory for `jsonl_file` (default `.mergecraft/traces/`). |

- | `tokenRef` | string | Reference to a secret (Batch D resolves the value). |

- | `project` | string | Project / namespace for `logfire`. |

- | `endpoint` | string | OTLP / collector endpoint. |

- | `headers` | map[string]string | Optional headers for OTLP / collectors. |

-

- `tokenRef` is never inlined (D5). `path` is repo-relative when the sink

- operates inside the Action container.

-

- ## Sink types

-

- | Type | Implementation | Status |

- | ------------ | ------------------------ | -------------- |

- | `jsonl_file` | `JSONLFileSink` | Batch A (W2) |

- | `memory` | `MemorySink` | Batch A (W2) |

- | `logfire` | OTLP exporter (one path) | Batch D (W8) |

- | `otel` | OTLP exporter (one path) | Batch D (W8) |

- | `sqlite` | deferred (D10) | not scheduled |

-

- ### `jsonl_file` — daily rotation, 30-day retention

-

- One JSONL file per UTC day, named `YYYY-MM-DD.jsonl`, written under the

- configured `path`. Default retention is 30 days; `purge_expired()` removes

- files whose mtime is older than the cap.

-

- ### `memory` — in-process only

-

- Records every event in a `list`; available for tests and short-lived

- fixtures. Nothing escapes the process.

-

- ### `logfire` / `otel` — one path (D5)

-

- Both resolve to the same OTLP exporter behind a batch processor. The

- remote sink contract is owned by W8.

-

- ## Redaction guarantee (D7)

-

- Redaction runs **once**, **before** fan-out. Every event reaches every

- sink through a `RedactingSink` wrapper around `MultiSink` — no sink is

- ever reachable without going through the redaction boundary.

-

- - The implementation **reuses** `src/mergecraft/utils/secrets.py`

- (`filter_env`, `is_sensitive_env_name`) and

- `src/mergecraft/analyzers/redact.py` (`redact_secrets`). No second

- matcher is implemented.

- - A deny-key list (`authorization`, `cookie`, `api_key`, `secret`,

- `password`, `access_token`, `refresh_token`, `id_token`, `bearer_token`,

- `auth_token`) — case-insensitive — replaces matching attribute values

- with `<redacted>`.

- - The shared helper applies to every string attribute value (recursively

- into nested dicts and lists) so `ghp_…` and `sk-…` substrings cannot

- escape.

-

- ## Payload cap (D8)

-

- `TRACE_ATTRS_JSON_MAX_BYTES = 64 * 1024`. When any single string value in

- an event's `attrs` exceeds the cap, the row is written with

- `attrs = {"truncated": True}` instead. The row survives on disk and

- downstream consumers see the marker rather than a missing or half-written

- record.

-

- ## Retention (D8)

-

- `retentionDays` (default `30`) governs `JSONLFileSink.purge_expired()`. Files

- whose mtime is older than the cap are removed on the next write (or

- explicit purge).

-

- ## Limits

-

- Two independent caps bound how much a single run can emit — both are guards

- against a runaway, not budgets to plan around, and neither fails the run

- when hit (convention 6).

-

- | Cap | Constant | Value | Behaviour past the cap |

- |---|---|---|---|

- | Per-event `attrs` size | `TRACE_ATTRS_JSON_MAX_BYTES` (`src/mergecraft/tracing/cap.py:18`) | 64 KiB | `cap_event_attrs()` replaces `attrs` with `{"truncated": True}`; the row still lands. |

- | Span count per run | `MAX_SPANS_PER_RUN` (`src/mergecraft/tracing/tracer.py`) | 10,000 | `Tracer.start_span()` keeps returning a span (so callers don't need to special-case it), but the span is suppressed — it never reaches the configured sink. Logged once at `warning` with the count on the first span past the cap, not once per subsequent call. |

-

- 10,000 is roughly 20x the realistic ceiling for a large review (one

- `analyzer.run` per analyzer, one `tool.call` per tool invocation, one

- `llm.call` + `provider.call` per turn), so it only fires on a genuine

- runaway — a large PR review should never come close. If a legitimate run

- does hit it, raise the `Final` constant; it is a single line to change.

-

- ## Behaviour guarantees

-

- 1. **Disabled is a no-op** (convention 9). A repo that does not enable

- tracing never touches the filesystem; the `attrs_source` callable on

- a `NullSink.emit` is never invoked.

- 2. **Tracing never fails the run** (convention 6). A sink that raises on

- `write` is caught and logged at `logger.warning`; the run continues.

- 3. **Optional extra** (D6). `logfire` and `opentelemetry-*` are not base

- dependencies. `pip install merge-craft[tracing]` pulls them in;

- `make ci-resume` passes with them uninstalled (convention 5).

- 4. **No network in `make ci-resume`** (convention 8). Exporter tests

- target a fake transport.

-

- ## D15 — remote sinks export reviewed-repo content

-

- > Enabling a **remote** sink (`logfire`, `otel`) exports reviewed-repo

- > content — the prompts the reviewer received, the tool inputs and

- > outputs it produced, and the model's reasoning — to the configured

- > endpoint, using the operator's token or API key. **BYOK** means the

- > operator owns both the credential and the responsibility for what

- > leaves the runner. Scope workflows at the GitHub Actions level

- > (`if: github.event.pull_request.head.repo.fork == false`) when the

- > reviewer should not exfiltrate fork-PR content.

-

- There is no config-level trust gate. D15's hard requirement is that the

- statement above appears plainly in this document, so the operator sees

- it the first time they reach for a remote sink.

-

- ## Content-capture policy for model payloads (OB2 — D6/D7/D8)

-

- D15 warns that remote sinks export reviewed-repo content; the `content`

- policy is the **level control** that decides how much of it leaves the

- runner. It governs model payloads (prompts, completions, reasoning) via

- `tracing/content.py`, with four levels (D6):

-

- | Level | Body | Metadata (`.chars` / `.bytes` / `.sha256`) | Use |

- | --- | --- | --- | --- |

- | `off` | — | — | Nothing is captured, hash included |

- | `metadata` | — | ✓ | Counts + hash only — the untrusted-tier ceiling |

- | `redacted` (default) | ✓, through the secret matcher (`analyzers.redact.redact_secrets`), capped | ✓ | Safe default |

- | `full` | ✓, verbatim, capped only | ✓ | Local debugging |

-

- Resolution (`resolve_content_capture(configured, trust_tier)`):

- `MERGECRAFT_TRACING_CONTENT` env → the YAML `tracing.content` field → the

- default `redacted`. An unrecognised value at any step falls through to the

- next, ending at the default — fail safe, never `full`. Bodies are

- byte-capped at the shared `TRACE_ATTRS_JSON_MAX_BYTES` budget and flagged

- `.truncated`; `.chars` / `.bytes` / `.sha256` always describe the

- **original** payload (D8), so the hash detects prompt drift between two

- runs even when neither shipped a body.

-

- **D7 — untrusted runs cap at `metadata` unless you opt in twice.** At any

- trust tier other than `trusted`, a body-emitting level is lowered to

- `metadata` **after** precedence resolution. `content: full` in YAML and

- `MERGECRAFT_TRACING_CONTENT=full` both yield `metadata` on a fork-PR-shaped

- run. The cap only ever lowers a level — `off` stays `off`, and nothing is

- ever raised.

-

- To ship prompt/completion bodies from an untrusted run to a sink **you

- own**, set a second explicit **operator-owned** knob as well as a

- body-emitting level. Fork-controlled `.mergecraft/config.yaml` on the PR

- head cannot lift the cap — that file is attacker-controlled on a fork PR.

-

- Operator-owned sources, in order:

-

- 1. Action input `tracing-export-untrusted-content: "true"` (workflow from

- the base on `pull_request_target`)

- 2. Env `MERGECRAFT_TRACING_EXPORT_UNTRUSTED_CONTENT=true` / CLI

- `--tracing-export-untrusted-content`

- 3. YAML `tracing.exportUntrustedContent: true` on a **trusted** run, or

- on the run-start snapshot of a `pull_request_target` job (the base

- checkout — not the fork HEAD after `checkout_pr`)

-

- Source (3) is gated by the snapshot's own `operator_owned` provenance

- (`RepoSettingsSnapshot.operator_owned`, set once when the snapshot is

- captured, from the same event name that derived `trust_tier` — never a

- second, independent read of `GITHUB_EVENT_NAME`). A snapshot with no

- provenance, or one rebaselined after `checkout_pr` with no prior snapshot

- to carry it forward from (a live load off whatever is on disk, which may

- already be the fork's own HEAD), is never operator-owned — it falls

- through to `None`, i.e. only (1) or (2) can lift the cap.

-

- ```yaml

- tracing:

- enabled: true

- to: logfire

- content: full

- exportUntrustedContent: true

- ```

-

- `content: full` alone still does not lift the cap — shipping a fork PR's

- prompt text to a remote sink is the exfiltration path trust tiers exist

- to close. Codex/Claude/Gemini CLI traces stamp the prompt mergeCraft

- already built (the wire request is not visible to those harnesses);

- OpenCode HTTP still sees the live request.

-

- ## Span tree (W4 — Batch B)

-

- Every tracing-enabled run emits one **root span** (`mergecraft.run`) with

- the run lifecycle, and a fixed set of **child spans** at the existing

- seams. Spans carry `parent_span_id` pointers so the tree is reconstructible

- from any sink's flat event stream. The root is the only span whose

- `parent_span_id` is `None` — convention 9.

-

- ```text

- mergecraft.run (root; run_id, repo, pr_number,

- commit_sha, workflow_run_id, job_id)

- ├── mergecraft.prep (toolchain install: language servers,

- │ linters, action deps)

- ├── mergecraft.analyzers.pipeline (W7: detect, run, scope, cluster,

- │ │ budget — analyzer fan-out)

- │ └── analyzer.run ×N (analyzer.id, exit_code,

- │ findings_count, duration_ms,

- │ skipped, error)

- ├── agent.attempt ×N (per fallback entry; model.id,

- │ │ agent.provider, agent.mode,

- │ │ agent.cli_argv — redacted,

- │ │ model.fallback_index, status,

- │ │ error)

- │ └── llm.call (wraps the agent attempt: duration,

- │ │ gen_ai.input.messages /

- │ │ gen_ai.output.messages when capture

- │ │ policy allows, plus token usage)

- │ └── tool.call ×N (tool.name, tool.server — same trace)

- └── mergecraft.publish (finalisation: persist learnings,

- report status checks, emit packet)

- ```

-

- ### Attributes per kind

-

- | Kind | Required attributes (issue §4 + W4.4) |

- | ----------------------------- | ---------------------------------------------------------------------------------------------------- |

- | `mergecraft.run` | `run_id`, `repo`, `pr_number`, `commit_sha`, `workflow_run_id`, `job_id` (correlation from env or kwarg) |

- | `mergecraft.prep` | (no required attrs beyond span defaults) |

- | `mergecraft.analyzers.pipeline` | (no required attrs beyond span defaults) |

- | `analyzer.run` | `analyzer.id`, `analyzer.exit_code`, `analyzer.findings_count`, `analyzer.duration_ms` |

- | `agent.attempt` | `model.id`, `agent.provider`, `agent.mode`, `agent.cli_argv` (redacted), `model.fallback_index`, `status` |

- | `llm.call` | `cost.tokens_in`, `cost.tokens_out`, `cost.cache_read`, `cost.cache_write`, `cost.usd` |

- | `tool.call` | `tool.name`, `tool.server` |

- | `mergecraft.publish` | (no required attrs beyond span defaults) |

-

- ### Defaults and guarantees

-

- - The default is **off** (convention 9). When `tracing.enabled` is `false`,

- `get_tracer_from_settings` returns a `NullTracer` whose `start_span`

- returns a `NullSpan` that short-circuits every emit — no sink is touched,

- no `attrs_source` callable is invoked, no directory is created.

- - The tracer is **never on the critical path** (convention 6). Any

- exception inside an emit site is caught by `MultiSink.write` and logged

- at `logger.warning` with the sink type and message; the run continues.

- - **Redaction runs once, before fan-out** (D7). Every emit traverses a

- `RedactingSink` wrapper around `MultiSink`. The `MemorySink` that

- structural tests use also redacts on write, so test assertions and the

- production path see the same surface.

- - **Correlation fields** are read from the `correlation` kwarg at the

- emit site if provided; otherwise they are derived from `GITHUB_*` env

- vars (`GITHUB_RUN_ID`, `GITHUB_REPOSITORY`, `GITHUB_PR_NUMBER`,

- `GITHUB_SHA`, `GITHUB_JOB`). Local dev runs that lack those vars get

- safe placeholders rather than `None`.

-

- ## Per-driver streaming coverage (W6 — Batch C)

-

- The W6 read-loop migration replaces `subprocess.run(..., capture_output=True)`

- with `subprocess.Popen` + a line-buffered `consume_stream` consumer that

- emits a `tool.call` or `llm.call` span per event. The exact span surface

- depends on what the upstream CLI emits; this table pins the version the

- plan was authored against and the per-event coverage each driver

- delivers today.

-

- | Driver | CLI version pinned (W0.5) | Streaming flag | Coverage |

- | -------- | ------------------------- | --------------------------------------------- | ----------------------------------------------------------------------------------------- |

- | `claude` | Claude Code 2.1.226 | `--print --output-format stream-json` | **Per-event**: one `llm.call` per `message_start`/`message_stop`; one `tool.call` per `content_block_start`/`stop`. Authoritative usage from the final `result` event. |

- | `codex` | codex-cli 0.146.0 | `codex exec --json` | **Per-event**: one `llm.call` per `thread.started`; one `tool.call` per `item.started`/`item.completed`. Authoritative usage from the `turn.completed` event. |

- | `gemini` | gemini-cli 0.53.0 | `-p <prompt> --output-format stream-json` | **Per-event**: one `llm.call` per `init`; one `tool.call` per `tool_use`/`tool_result`. Final usage from the `result` event. |

- | `opencode` | opencode 1.18.13 | `opencode run --format json` (HTTP via serve) | **Run-level only** — opencode's events are partial (W0.5), so the driver degrades to a `run`-level span per `agent.attempt`. The HTTP polling path emits no NDJSON stream to consume. |

- | `cursor` | (cloud) | n/a — Cursor Cloud HTTP polling | **Run-level only** — no local streaming; the driver intentionally does not change its read path (W6.4). |

-

- Graceful degradation is the contract: a driver that cannot stream (or

- whose CLI's event shapes are not granular enough) emits run-level

- spans rather than failing. The `test_non_streaming_driver_degrades_to_run_level`

- and `test_cursor_degrades_to_run_level` regression pins in

- `tests/tracing/streaming/test_driver_degradation.py` enforce this.

-

- Malformed events are skipped and counted: the consumer never raises on

- a bad line. The counter is available via `StreamSpanAccumulator.malformed_event_count`

- and the line is logged at `warning` level.

-

- ## Provider and HTTP spans (T2)

-

- Every outbound provider request surfaces as a `provider.call` row with

- the transport family on it (`provider.transport_family` is one of

- `anthropic` / `responses_api` / `chat_completions`). The driver's

- existing `llm.call` span becomes a child of `provider.call`, and every

- outbound `httpx` call the driver made (e.g. the OpenAI-compatible

- custom-provider POST) becomes a grandchild via `instrument_httpx`.

-

- The Logfire tree therefore matches the sevn reference shape:

-

- ```

- agent.attempt

- ├── provider.call (provider.transport_family=...)

- │ ├── http.client.request (http.method, http.url-redacted, status)

- │ └── llm.call (model.id, cost.*, gen_ai.usage.*)

- ├── tool.call

- └── ...

- ```

-

- - **When does `provider.call` fire?** Once per upstream API request.

- Claude fires on `message_start` / closes on `message_stop`; Codex on

- `thread.started` / `turn.completed`; Gemini on `init` / `result`. The

- span exists so Logfire groups rows by transport family.

- - **When does `http.client.request` fire?** Once per outbound `httpx`

- `send()` on a wrapper mergeCraft constructed (D8 — no global monkey

- patch). The wrapper installs on the two `httpx.AsyncClient` instances

- `agents/opencode.py::_prompt_session` and `agents/opencode.py::_run`

- use for the custom OpenAI-compatible provider path, the only httpx

- sites in the repo.

- - **`http.url` is always redacted inline** (D9 — see the URL redaction

- table below).

-

- ### URL redaction table

-

- `mergecraft.tracing.redaction.redact_url(url)` masks credential-shaped

- fragments while preserving the URL shape. Applied in order; first

- match wins per region:

-

- | Pattern | Before | After |

- |---------|--------|-------|

- | Telegram bot token | `https://api.telegram.org/bot123456:ABC/sendMessage` | `https://api.telegram.org/bot<redacted>/sendMessage` |

- | Basic auth | `https://user:pass@example.com/path` | `https://user:<redacted>@example.com/path` |

- | Query token (`api_key`, `access_token`, `token`, `key`, `secret`) | `https://example.com/v1/messages?api_key=sk-abc&x=1` | `https://example.com/v1/messages?api_key=<redacted>&x=1` |

- | Bearer header value | `Bearer ghp_longtoken123…` | `Bearer <redacted>` |

- | Embedded `sk-` / `ghp_` / `eyJ…` substring | `…sk-abc123def…` | `…<redacted>…` |

-

- The literal marker is `mergecraft.tracing.redaction.REDACTED = "<redacted>"`.

- The URL stays parseable (`urllib.parse.urlparse` round-trips on every

- shape), and non-token query parameters are preserved so the path-based

- grouping in Logfire's row inspector keeps working.

-

- ## Tool call attributes (T1)

-

- Every `tool.call` span carries the request/response byte counts,

- `exit_code`, error class/message, and input-key list sevn splits across

- `tool.invoke` / `tool.complete`. The shape is additive on the post-#137

- tree (D5: one enriched `tool.call` span, not sevn's `tool.invoke` /

- `tool.complete` split) so the existing `tool.name` / `tool.id` /

- `tool.server` / `gen_ai.*` attrs remain on the same row. The

- `src/mergecraft/tracing/_tool_attrs.py` helpers expose the open-side

- `enrich_tool_request` and the close-side `enrich_tool_response`

- (W4 / M1 split the legacy single `enrich_tool_call_attrs` into two

- single-purpose calls), plus `emit_verb_subevent` so the three drivers

- (`claude` / `codex` / `gemini`) and the MCP `tools/call` handler all

- emit the same shape.

-

- ### `tool.call` attributes

-

- | Attribute | Type | Example | Source |

- |-----------|------|---------|--------|

- | `tool.name` | str | `"browser"` | existing — preserved |

- | `tool.id` | str | `"tool-claude-1"` | existing — preserved |

- | `tool.server` | str | `"claude"` / `"codex"` / `"gemini"` / `"mergecraft"` | existing — preserved |

- | `tool.input` | dict / str | `{"q": "hello"}` / `"codex-input"` | existing — preserved |

- | `tool.output` | any | `"claude-output-text"` | existing — preserved |

- | `tool.arguments` | dict | `{"q": "hello"}` | T1 — request-side raw args (MCP server) |

- | `tool.argument_count` | int | `1` | T1 — request-side count |

- | `tool.argument_bytes` | int | `15` | T1 — request-side JSON-encoded size |

- | `tool.input_keys` | list[str] | `["q"]` | T1 — sorted key list (dict input only) |

- | `tool.input_bytes` | int | `15` | T1 — driver-side request byte count |

- | `tool.exit_code` | str | `"ok"` / `"error"` | T1 — success / failure marker |

- | `tool.result_kind` | str | `"text"` / `"json"` / `"image"` / `"list_of_blocks"` / `"unknown"` | T1 — MCP server success path |

- | `tool.result_bytes` | int | `18` | T1 — MCP server success path |

- | `tool.output_kind` | str | `"text"` / `"json"` / `"image"` / `"list_of_blocks"` / `"unknown"` | T1 — driver-side response classification |

- | `tool.output_bytes` | int | `18` | T1 — driver-side response byte count |

- | `tool.error_class` | str | `"RuntimeError"` | T1 — failure path only |

- | `tool.error_message` | str | `"tool kaboom: …"` (redacted + capped) | T1 — failure path only |

- | `gen_ai.operation.name` | str | `"execute_tool"` | existing — preserved |

- | `gen_ai.tool.name` | str | `"browser"` | existing — preserved |

- | `gen_ai.tool.call.id` | str | `"ab12cd…"` | existing — preserved |

- | `gen_ai.tool.input` | str | `"<redacted>"` | T1 — `redact_tool_payload` of the input |

- | `gen_ai.tool.output` | str | `"<redacted>"` | T1 — `redact_tool_payload` of the output (success or error message) |

-

- ### Verb sub-events (`tool.browse` / `tool.search` / …)

-

- Known-verb tools — `browser`, `search`, `read_file`, `write_file`,

- `run_code`, `load_tool` — also emit a verb-specific child span on the

- `tool_result` / `item.completed` close event. The mapping is the closed

- `KNOWN_VERB_TOOLS` dict in `src/mergecraft/tracing/_tool_attrs.py`:

-

- | Tool name | Child span kind |

- |-----------|----------------|

- | `browser` | `tool.browse` |

- | `search` | `tool.search` |

- | `read_file` | `tool.read` |

- | `write_file` | `tool.write` |

- | `run_code` | `tool.run_code` |

- | `load_tool` | `tool.load_tool` |

-

- The child span's `parent_span_id` is the parent `tool.call`'s `span_id`,

- and its attrs mirror the parent's so Logfire's row inspector still has

- full context for each verb row. Lifecycle: opened on the close event,

- closed immediately — no new bookkeeping state. Tools outside the closed

- set (a hypothetical `frobnicate`) emit only the parent `tool.call` and

- no child.

-

- ### Cap and redaction behaviour

-

- `tool.arguments` is capped at `TRACE_ATTRS_JSON_MAX_BYTES` (64 KiB) via

- the existing `cap_event_attrs` path: a value past the cap collapses

- the row's `attrs` to `{"truncated": True}` so the JSONL line stays

- parseable. `tool.output` is stringified + redacted via

- `mergecraft.tracing.redaction.redact_tool_payload(payload)` — the helper

- runs `json.dumps(default=str)` on non-str values, caps at 64 KiB

- (returning `"<truncated>"` on overflow), and pipes the result through

- `redact_secrets` so embedded tokens (`ghp_…` / `sk-…` / bearer headers)

- cannot escape onto the span. The same helper replaces the local

- `_truncate_tool_payload` copies in `agents/claude.py` and

- `agents/codex.py` so every driver + the MCP server share one source of

- truth.

-

- ## One trace per run (T3)

-

- One `mergecraft review` run emits one Logfire trace. Every span

- emitted by the run — `mergecraft.run`, `mergecraft.prep`,

- `mergecraft.publish`, `mergecraft.analyzers.pipeline`, `analyzer.run`,

- `agent.attempt`, `llm.call`, `tool.call`, plus the future `provider.call`

- and `http.client.request` — shares one Logfire `trace_id`. The UI groups

- spans by `trace_id` (the OTel `trace_id` field on the produced span), so

- a single click in the Logfire tree shows the full run — a feature the

- pre-#137 tree did not deliver because `mergecraft.trace_id` was just an

- attribute on every span rather than the OTel `trace_id` itself.

-

- ### What `trace_id` is

-

- `trace_id` is the Logfire / OpenTelemetry trace identifier shared by every

- span in one run. The mergeCraft run resolves it once per process (via

- `resolve_trace_id()` in `tracing/tracer.py`) and propagates it onto every

- child span via the `Tracer` (`self.trace_id`) and the `Span`

- (`self.trace_id`). The OTel exporter (`OTLPSink`) forwards it as the

- real OTel `trace_id` on the produced span so Logfire groups by it

- automatically — no attribute search required.

-

- ### How it is generated

-

- The resolver follows the same precedence as the existing session-id

- resolver (D7 / T3.2):

-

- 1. `MERGECRAFT_TRACE_ID` — explicit per-run override.

- 2. `MERGECRAFT_TRACE_SESSION_ID` — alias preserving the pre-#137 contract

- so existing pipelines keep working.

- 3. `GITHUB_RUN_ID` — the Actions run id, monotonic and unique.

- 4. `uuid.uuid4().hex` — local fallback when no env vars are set.

-

- `session_id` remains the per-process correlation id (the W4 batch-B

- session correlation) and `turn_id` is the per-span `uuid4().hex`. The

- three fields are orthogonal: change the env precedence and the run still

- groups under one trace.

-

- ### How Logfire groups by it

-

- `OTLPSink.write` rewrites the OTel `trace_id` on the produced span via

- `SpanContext(trace_id=otel_trace_id, …)` (the same private `_context`

- field the OTel SDK uses internally). The recording-processor test seam

- captures the rewritten `trace_id` so the Logfire-grouping contract is

- observable through the existing surface. The

- `otel_bridge.attach_trace_context` context manager does the same on the

- OTel **context** side, so any nested OTel auto-instrumented operation

- (e.g. an `httpx` call inside a tool) inherits the same `trace_id`

- without the caller having to know about mergeCraft's tracer. The

- `attach_trace_context` integration in

- `agents/_stream_consumer.py::consume_stream` wraps the handler call so

- any nested OTel operation inside an agent's per-event handler inherits

- the run's trace.

-

- ## Three identifiers: review.id, trace_id, review.correlation_key (OB1)

-

- One logical review fans out into several processes: the orchestrating run

- plus one spawned agent CLI per subagent. Three identifiers — not two —

- describe that shape (D2):

-

- | Identifier | Scope | Source |

- | --- | --- | --- |

- | `review.id` | **One logical review**, across every process and agent run | `tracing/review_context.py::resolve_review_id()` — `MERGECRAFT_REVIEW_ID` inherited verbatim, else a fresh `uuid4` per review |

- | `trace_id` | **One agent run** (one process) | `tracing/tracer.py::resolve_trace_id()` — see *One trace per run (T3)* above |

- | `review.correlation_key` | **Every attempt at one commit** — deliberately collides | `correlation_key_for()` — deterministic `sha256(repo\|pr\|head_sha)` (D3) |

-

- The shape to remember: **one review with three agent runs has one

- `review.id` and three `trace_id` values.** One `review.id` filter returns

- the entire review — every agent, every tool call, the verdict — across

- every process. `review.correlation_key` answers the orthogonal query:

- "every attempt at this commit", because two reviews of one commit are two

- reviews (distinct `review.id`s) but share the key (D3). A local patch

- review has no repo/pr/head context, so its key is empty and the attribute

- is omitted rather than emitted as a misleading constant.

-

- ### How the identity travels

-

- - **Within a process:** both entry points — the CLI

- (`offline_review.py::run_offline_diff_review`) and the Action

- (`main.py::main`) — bind a frozen `ReviewContext` via

- `bind_review_context(...)`. `Span.close()` reads the bound context at

- **close time** (D4), so a context bound after the tracer was built still

- reaches spans that are already open. Merge precedence: tracer baseline →

- review context → lazy `attrs_source` → explicit `set_attribute`.

- - **Across the process boundary (O2):** `agents/shared.py::spawn_agent_cli`

- — the single choke point for all five drivers — exports

- `MERGECRAFT_REVIEW_ID` + `MERGECRAFT_REVIEW_CORRELATION_KEY` into the

- child env via `setdefault`, after the privilege-drop env patch. A

- driver-pinned value wins; a fail-closed `setpriv` error still surfaces

- first. The child's `resolve_review_id()` then inherits the parent's

- review verbatim.

- - **Baseline attrs (O3):** `baseline_run_attrs()` stamps every span with

- `mergecraft.version`, `mergecraft.run_id`, `mergecraft.trust_tier`, and

- the VCS/CI fields (`vcs.repository.name`, `vcs.change.id`,

- `vcs.revision`, `ci.workflow_run_id`, `ci.job_id`) so a span can say

- which build and which change produced it. The `Tracer` carries them in a

- `baseline_attrs` field with `repr=False` (D5).

-

- ## What's next

-

- | Batch | Wave | Scope |

- | ----- | ----- | ---------------------------------------------------- |

- | C | W6 | `stream-json` migration for per-tool spans |

- | D | W8 | `logfire` + `otel` exporters, CLI / action inputs, complete docs (DONE) |

-

- ## Self-hosted endpoints (W8.1 / D5)

-

- The `otel` sink accepts any OTLP/HTTP collector URL. The token / API key

- travels as an `Authorization: Bearer …` header; the `headers` map on the

- config entry can carry extra static headers for proxies, tenants, or

- custom routing.

-

- ```yaml

- tracing:

- enabled: true

- sinks:

- - type: otel

- endpoint: https://otel.internal.example.com:4318/v1/traces

- headers:

- x-tenant: mergecraft

- ```

-

- For Logfire, the endpoint is the region-aware OTLP/HTTP ingest URL —

- `https://logfire-us.pydantic.dev/v1/traces` (US) or

- `https://logfire-eu.pydantic.dev/v1/traces` (EU), selected by the sink's

- `region` field (default `us`). The `project` field is informational only;

- Logfire routes spans by the token itself, so no `x-logfire-project` header

- is sent. The token is resolved through `tokenRef` (D5) — see *Token

- resolution* below.

-

- ## Token resolution (W8.2 / D5)

-

- `logfire` tokens are referenced by name, never inlined. The factory

- resolves a `tokenRef` against `os.environ` at run time; when the

- reference is unset, the resolver falls back to the canonical

- `MERGECRAFT_LOGFIRE_TOKEN` env var. The resolved value is held in

- runtime memory only — it never appears in config dumps, YAML

- round-trips, or the `mergecraft config tracing` output. The CLI

- renders the value as `*** (redacted)` even in the table form.

-

- When the token cannot be resolved, the sink is constructed but emits a

- warning and degrades to a no-op for the network path. The local

- `jsonl_file` sink (when configured) keeps writing.

-

- On the Action path, tracing enabled with `tracing-to: logfire` but no

- resolvable token also surfaces in the **job step summary** and run record

- via `collect_tracing_warnings_for_summary()` — a configured-but-inactive

- Logfire sink is no longer silent. The warning names `logfire-token`,

- `INPUT_LOGFIRE_TOKEN`, and `MERGECRAFT_LOGFIRE_TOKEN`.

-

- ## Action inputs (W8.5 / W7.7)

-

- `action.yml` exposes tracing inputs so a consuming repo can wire tracing

- without touching `.mergecraft/config.yaml`:

-

- | Input | Maps to |

- | ------------------------------------ | ---------------------------------------------------- |

- | `tracing` | `tracing.enabled` (overrides config) |

- | `tracing-to` | `tracing.to` shorthand (overrides config) |

- | `logfire-token` | resolved logfire token (D5 — held at runtime only) |

- | `otel-endpoint` | `tracing.sinks[].endpoint` for the `otel` sink type |

- | `tracing-content` | `tracing.content` (`off` / `metadata` / `redacted` / `full`) |

- | `tracing-export-untrusted-content` | operator D7 lift (env / Action; fork HEAD YAML cannot) |

-

- The Action wraps `${{ secrets.LOGFIRE_TOKEN }}` into `logfire-token`

- so the secret never appears in the workflow file.

-

- ## CLI surface (W8.4 / W7.6)

-

- ```text

- mergecraft review --tracing|--no-tracing [--tracing-to <shorthand>] \

- [--trace-dir <path>] [--logfire-token <token>] \

- [--otel-endpoint <url>] \

- [--tracing-content off|metadata|redacted|full] \

- [--tracing-export-untrusted-content]

- mergecraft config tracing # render resolved state with token redacted

- mergecraft traces <run-id> # read back local JSONL spans for a run id

- ```

-

- The precedence order is **CLI flag > env var > `.mergecraft/config.yaml`

- > default (off)**. `--no-tracing` wins over any lower-precedence

- `true`. The full table is asserted by `tests/tracing/exporters/test_cli_precedence.py`.

-

- `mergecraft config tracing` does **not** require the `tracing` extra

- — it operates on resolved settings, not on the live exporters.

-

- ## Artifact upload (W8.6 / D14)

-

- The Action writes local traces under `.mergecraft/traces/`. A typical

- workflow ships them out of CI with `actions/upload-artifact@v4`:

-

- ```yaml

- - uses: alexhawat/mergecraft@<ref>

- with:

- tracing: "true"

- tracing-to: local_files

- - uses: actions/upload-artifact@v4

- if: always()

- with:

- name: mergecraft-traces

- path: .mergecraft/traces/

- ```

-

- `if: always()` ensures the artifact is uploaded even when the review

- exits non-zero — the trace is the most useful when the run failed.

- The path is `.mergecraft/traces/` by default; override with the

- `trace_dir` config field or the `--trace-dir` flag.

-

- ## Structured logs (operator debugging, W12.6 / #33)

-

- Tracing owns spans; default Loguru output stays human-readable. For

- correlation fields in the log stream itself (without enabling tracing),

- opt into JSON:

-

- ```bash

- export MERGECRAFT_LOG_FORMAT=json # or LOG_FORMAT=json

- # optional: LOG_LEVEL=DEBUG

- ```

-

- When JSON is on, each record includes bound context when available:

-

- | Field | Source |

- |-------|--------|

- | `run_id` | `GITHUB_RUN_ID` (bound in `main`) |

- | `repo` | `owner/name` for the run |

- | `pr` | pull-request number when the event carries one |

- | `phase` | coarse run phase (`setup`, …) |

-

- Bind or refresh fields from code with

- `mergecraft.utils.log.bind_run_context(...)`. Use tracing (above) when you

- need model/tool span trees; use JSON logs when you need a grep-friendly stream

- of the same run on the runner console or log drain.

-

- ## Agent stream log surface (plan 13 W6–W7)

-

- This section is **not** tracing spans — it is what operators see on the Actions

- console while the reviewing agent runs.

-

- | Before (plan 13) | After |

- |------------------|-------|

- | Loguru → stderr, stream echo → stdout — unsynchronised writers, interleaved mid-line | One writer: Loguru with `enqueue=True`; stream renderer emits through the same sink |

- | Full provider NDJSON dumped to stdout on every stream event | Human-readable lines from `agents/stream_render.py` (`emit_rendered_stream_line`) |

- | Forensic detail only in a masked, interleaved dump | Evidence packet `trajectory` built from MCP tool state (see [`evidence-packet.md`](evidence-packet.md)) |

-

- **Raw NDJSON is not the default log surface.** `consume_stream` still parses each

- line for spans and trajectory, but operator-facing output is rendered. The raw

- line is logged at `DEBUG` only when `is_debug_enabled()` is true:

-

- - `LOG_LEVEL=debug`, or

- - `ACTIONS_STEP_DEBUG=true` (GitHub Actions step debug)

-

- Until plan 12's artifact upload lands, raw NDJSON is not uploaded as a separate

- artifact — enable debug on the step when you need line-level provider JSON in

- the job log. The trajectory record in the evidence packet is the durable

- forensic surface for tool identity, outcomes, paths, and failure class.

-

- Loguru queue drain is registered beside the existing temp-dir `atexit` hook so

- Docker action tails are not lost when the container exits.

-

- ===== FILE: docs/install.md =====

-

- # Installing mergeCraft

-

- Consumer install paths for the GitHub Action and the local CLI. The landing

- [Example 1](../README.md#example-1--auto-review-every-pr) workflow is the

- minimal auto-review setup.

-

- ## Requirements

-

- - **Python 3.11+** when installing the CLI locally ([`docs/dev/python-version-floor.md`](dev/python-version-floor.md))

- - [uv](https://docs.astral.sh/uv/) for `uv tool install`

- - An authenticated [GitHub CLI](https://cli.github.com) (`gh auth login`) for `mergecraft init` and `mergecraft provider auth`

- - One provider credential (Claude Pro/Max, ChatGPT Plus/Pro, or an API key)

-

- mergeCraft **0.1.0** supports **GitHub** repositories only. GitLab support is

- planned via the `ScmProvider` abstraction.

-

- ## Path A — GitHub Action (no local Python)

-

- Add a workflow that uses the Docker Action. Pin to an immutable ref — a git tag

- when one exists, or a full commit SHA until the first release tag is cut

- ([`docs/distribution.md`](distribution.md)).

-

- See [Example 1 in the README](../README.md#example-1--auto-review-every-pr).

-

- For a pinned runtime without managing Python versions, the container image ships

- a compatible interpreter; no local Python install is needed.

-

- ## Path B — local CLI

-

- Install from git (PyPI is not published yet):

-

- ```bash

- uv tool install "merge-craft @ git+https://github.com/alexhawat/mergeCraft"

- mergecraft --install-completion # bash/zsh/fish — see --show-completion

- mergecraft init # writes .mergecraft/config.yaml + .github/workflows/mergecraft.yml

- ```

-

- Then authenticate and open a pull request — details in the README install steps

- and [`docs/authentication.md`](authentication.md).

-

- ## Docker-only consumers

-

- Use the Action workflow from Example 1 and set provider secrets in GitHub Actions.

- You do not need `uv tool install` on your laptop unless you want local

- `mergecraft review` runs.

-

- **See also:** [`docs/cli.md`](cli.md) · [`docs/action-reference.md`](action-reference.md) · [`docs/distribution.md`](distribution.md)

-

- ===== FILE: docs/authentication.md =====

-

- # Authentication

-

- ## Quick start — init → auth → review (D10)

-

- A new consumer repo can review immediately after two commands — no manual roster

- editing:

-

- ```bash

- mergecraft init

- mergecraft provider auth anthropic # or openai, google, nous, …

- mergecraft review # local review works now

- ```

-

- `mergecraft init` scaffolds `.mergecraft/config.yaml` and the workflow. The first

- successful `provider auth` seeds `agents.reviewer` p0 with that provider's

- preferred catalog model and inserts the slug into `models:` when needed. Inspect

- the result with `mergecraft agent list`.

-

- To override models for local runs only — without changing what CI sees — use

- `mergecraft agent-local` (see [`docs/agent-roster.md`](agent-roster.md)).

-

- Commit `.mergecraft/config.yaml` and `.github/workflows/mergecraft.yml` after

- auth. Run `mergecraft workflow sync --check` before pushing if you assign roster

- models whose providers are not yet wired in the workflow.

-

- This repository's workflow does not listen for `@mergecraft review` — reviews

- run on `pull_request_target` (opened / synchronize).

-

- ## Provider reference

-

- | Provider | Subscription (recommended) | API key | Recommended model | Inferred harness |

- |----------|-----------------------------|---------|-------------------|------------------|

- | Anthropic Claude | `mergecraft provider auth anthropic --scope github` → indexed credentials (Claude Pro/Max) | `ANTHROPIC_API_KEY` | `anthropic/claude-sonnet` | `claude` |

- | OpenAI Codex | `mergecraft provider auth openai --scope github` → indexed credentials (ChatGPT Plus/Pro/Team/Enterprise) | `OPENAI_API_KEY` | `openai/gpt-5.3-codex` | `codex` |

- | Google Gemini | `mergecraft provider auth google --scope github` → indexed credentials (AI Studio) | `GEMINI_API_KEY` / `GOOGLE_GENERATIVE_AI_API_KEY` | `google/gemini-3.1-pro-preview` | `gemini` |

- | Nous Portal | — (API key) | `mergecraft provider auth nous --scope github` → indexed credentials; `NOUS_API_KEY` remains a legacy input | `nous/deepseek/deepseek-v4-flash` | `opencode` |

- | Tencent TokenHub | — (API key) | `mergecraft provider auth tokenhub --scope github` → indexed credentials | `tokenhub/hy3` | `opencode` |

- | MiniMax | — (API key) | `mergecraft provider auth minimax --scope github` → indexed credentials | `minimax/MiniMax-M3` | `opencode` |

- | Cursor Cloud | `mergecraft provider auth cursor --scope github` → indexed credentials | `CURSOR_API_KEY` | `cursor/cloud-agent` | `cursor` |

- | OpenAI-compatible (custom) | — | `MERGECRAFT_CUSTOM_PROVIDER_BASE_URL` + `MERGECRAFT_CUSTOM_PROVIDER_API_KEY` (indexed `_1`/`_2` …) | `<your-prefix>/<your-model>` — see [Custom OpenAI-compatible provider](#custom-openai-compatible-provider) | `opencode` |

- | Logfire tracing | `mergecraft auth logfire` → `MERGECRAFT_LOGFIRE_TOKEN` + `MERGECRAFT_TRACING_PROJECT` (local) and `LOGFIRE_TOKEN` (Actions) | see [`docs/TRACING.md`](TRACING.md) | — | — |

-

- When `harness:` is unset in `.mergecraft/config.yaml`, mergeCraft infers the harness

- from the model slug (the **Inferred harness** column). Set `harness:` explicitly to

- override inference; unsupported combinations are configuration errors naming both halves

- (see [compatibility-matrix.md](compatibility-matrix.md) § Harness × model).

-

- Subscription auth runs the official `claude` / `codex` / `gemini` CLIs as *you*

- — the same credential your local coding agent uses. Only set env vars for

- providers you actually use.

-

- ### Turning a provider off

-

- `mergecraft provider disable <label>` is the inverse of `auth` / `provider auth`:

- it deletes the provider's GitHub Actions secret and/or blanks its `.env` entry,

- so CI stops selecting that provider on the next run.

-

- ```bash

- mergecraft provider disable nous # Actions secrets (default)

- mergecraft provider disable nous --scope both # Actions secrets + local .env

- mergecraft provider enable nous --scope both # re-authenticate

- ```

-

- It takes the same `--scope local|github|both` as `auth` and

- `mergecraft tracing logfire disable`, and the `auth` subcommand names work as

- labels (`provider disable codex` is `provider disable openai`). Both credential

- shapes are cleared — the flat secret the workflow names (`NOUS_API_KEY`) and the

- indexed registry key (`LLM_PROVIDER_<N>_API_KEY`), and every alias a provider

- recognises (`google` clears `GEMINI_API_KEY` *and*

- `GOOGLE_GENERATIVE_AI_API_KEY`) — so a provider is off however it was

- authenticated.

-

- "Disabled" is all-or-nothing. A provider stays usable through **any** of its

- credentials, so the command reports success only when every credential in the

- requested scope reaches the absent-or-blank state; if one cannot be cleared it

- exits non-zero and names what is left. A credential that was already absent

- counts as cleared.

-

- `--cwd` selects the repository the command acts on — both the `.env` it blanks

- and the `origin` whose Actions secrets it deletes — so it is safe to disable a

- provider in a repository you are not standing in. `MERGECRAFT_ENV`, when set,

- still names the `.env` explicitly.

-

- Disable clears **credentials**, not registration: the `providers:` row and its

- `LLM_PROVIDER_<N>` label survive, so `provider enable` re-authenticates the same

- env index. Use `mergecraft provider delete` to drop the registration itself.

- Workflow YAML is never rewritten — the shipped cascade already skips a provider

- whose secret is missing.

-

- > **Codex on container runners:** Codex CLI's nested bubblewrap sandbox fails

- > inside namespaced containers. On an already-isolated ephemeral runner, pass

- > `codex_sandbox: danger-full-access`. mergeCraft never sets this itself —

- > its own `shell`/`push` controls remain the security boundary

- > ([issue #70](https://github.com/alexhawat/mergeCraft/issues/70)).

- > Whether the override is honoured is decided by `trust.agentSandbox` — see

- > [`docs/trust-policy.md`](trust-policy.md).

-

- ### Where the local `.env` is resolved

-

- Every command that reads or writes a local credential resolves one of two

- anchors, and `MERGECRAFT_ENV` overrides both:

-

- | Command shape | Anchor |

- |---------------|--------|

- | Takes `--cwd` (`provider`, `model`, `agents`, `trust`) | `<cwd>/.env`, taken literally — the same directory `<cwd>/.mergecraft/config.yaml` is read from, so the registry and the credentials always name one repository |

- | Takes no `--cwd` (`auth`, `tracing logfire`, and the CLI startup load) | `<git-repo-root>/.env`, walking up from the process working directory |

-

- The walk-up is why `mergecraft auth` run from a subdirectory writes the `.env`

- the next invocation actually loads. Outside a git checkout the writers fail

- with the directory they consulted named in the error; the startup load falls

- back to `./.env` and stays silent when it is missing, so global invocations and

- CI sandboxes are unaffected.

-

- ### Credential detection (`credential_status_for_slug`)

-

- `has_credentials_for_slug` and `mergecraft provider status` delegate to a

- single probe — `credential_status_for_slug` in

- `mergecraft.utils.agent_resolve`. It returns:

-

- | Field | Meaning |

- |-------|---------|

- | `available` | Whether a usable credential was found for the model slug |

- | `source` | Which route won, or `None` when absent |

- | `looked_for` | Env var names consulted, for operator-facing skip messages |

-

- Four detection routes are consulted in order (first match wins):

-

- | `source` | When it applies |

- |----------|-----------------|

- | `registry-indexed` | `providers:` registry entry with a populated indexed credential env pair |

- | `gateway-singleton` | Singleton or indexed `MERGECRAFT_CUSTOM_PROVIDER_*` gateway env vars |

- | `cli-auth` | Subscription auth state for built-in providers (`claude`, `codex`, `gemini`, …) |

- | `legacy-env` | Flat per-provider secrets (`NOUS_API_KEY`, preset gateway env aliases) |

-

- When a roster slot is skipped for missing credentials, the run record and PR

- summary name the agent, slot, provider, and the env vars from `looked_for`.

- "Not wired in the workflow" and "env var empty" stay distinct messages —

- workflow wiring is `mergecraft workflow sync`; detection is this probe (#552).

-

- ### Custom OpenAI-compatible provider

-

- For any OpenAI-compatible endpoint (Nous Portal, Tencent TokenHub,

- MiniMax, OpenRouter, a [LiteLLM](https://docs.litellm.ai/) proxy, a self-hosted

- vLLM, etc.), mergeCraft exposes one mechanism that both harnesses consume.

- LiteLLM has no first-class provider id — point

- `MERGECRAFT_CUSTOM_PROVIDER_BASE_URL` at `https://<litellm-host>/v1` and

- `MERGECRAFT_CUSTOM_PROVIDER_API_KEY` at your proxy key; see

- [`examples/config-litellm.yaml`](../examples/config-litellm.yaml). Issue

- [#71](https://github.com/alexhawat/mergeCraft/issues/71) closes on this

- surface — the **Codex half** is new in `v0.0.x`; the OpenCode half

- shipped earlier in PR

- [#79](https://github.com/alexhawat/mergeCraft/pull/79) and is

- regression-tested.

-

- #### Env-var convention

-

- | Form | Example | Provider id |

- |------|---------|-------------|

- | Singleton back-compat alias (PR #79 / D7) | `MERGECRAFT_CUSTOM_PROVIDER_BASE_URL` + `MERGECRAFT_CUSTOM_PROVIDER_API_KEY` | `default` (or the active model's prefix when the model is `nous/...` or `tokenhub/...`) |

- | Indexed multi-provider | `MERGECRAFT_CUSTOM_PROVIDER_BASE_URL_1` + `MERGECRAFT_CUSTOM_PROVIDER_API_KEY_1`, `_2`, `_3`, … | `provider_1`, `provider_2`, `provider_3`, … |

-

- Indexed env vars are operator-locked — both halves of each numeric pair

- must be set with non-empty values; partial pairs are silently dropped.

- Discovery enumerates every matching suffix, sorts by numeric `N`

- ascending, and preserves gaps (no renumbering). When any indexed pair is

- set, the singleton is ignored.

-

- #### Action inputs (`with:`)

-

- For the common single-provider case, two top-level `with:` inputs map

- onto the singleton env vars — no need to name them in `env:`:

-

- ```yaml

- - uses: alexhawat/mergeCraft@v0.1.0a1

- with:

- model: default/your-model-id

- provider_base_url: https://api.example.com/v1

- provider_api_key_env: MY_PROVIDER_API_KEY # the NAME of an env var, not the key value

- env:

- MY_PROVIDER_API_KEY: ${{ secrets.MY_PROVIDER_API_KEY }} # wire the secret here

- ```

-

- `provider_api_key_env` is the **env-var name** that holds the key;

- mergeCraft reads that env var's value and re-exports it as

- `MERGECRAFT_CUSTOM_PROVIDER_API_KEY`. The resolved key value is never

- inlined into the workflow file and never logged (convention 7). For

- multi-provider setups, fall back to the indexed env-var form below —

- `with:` cannot enumerate multiple providers.

-

- See [docs/action-reference.md](action-reference.md) for the full input list

- (every `with:` key, literal defaults, and descriptions).

-

- Behavioural note: `setup_failure_policy`'s and `setup_timeout`'s literal

- `action.yml` default is an empty string (unset defers to the S1/D10 policy

- described below); the *effective* runtime default when left unset is

- `inconclusive` and `10m` respectively.

-

- - S1 / D10 — what a trusted-tier `setupScript` failure (non-zero exit **or**