Add Codex CLI provider for structured findings

.github/workflows/ci.yml

@@ -55,3 +55,13 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - run: bash scripts/i18n-deadkey-check.sh
+
+  spdx-check:
+    # ADR-0012: every Go source must carry the per-file SPDX header.
+    # This guard fails the build on any new file that drops it, keeping
+    # the 100% coverage reached in v0.9.0 from regressing.
+    name: SPDX header check
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - run: bash scripts/spdx-check.sh

CHANGELOG.md

@@ -8,6 +8,23 @@ and the project adheres to [Semantic Versioning 2.0.0](https://semver.org/spec/v
 > Tags prior to **v0.4.0** were cut in the private repository and produced no
 > public artifacts; the first publicly released version is v0.4.0.
 
+## [1.3.0]
+
+### Added
+- **New CLI-tool-backed provider: `codex-cli`.** Drives a locally
+  installed OpenAI Codex CLI (`codex`) as the review engine, joining
+  `claude-cli` and `gemini-cli`. Selectable via `--cli codex` or
+  `--provider codex-cli`; no API key needed (reuses the host CLI's auth).
+  Driven through `codex exec --sandbox read-only --skip-git-repo-check`
+  (non-interactive, read-only). Like the other CLI providers it is a plain-text emitter —
+  no structured findings, so `--json` / `--markdown` / `remote pr` do not
+  apply.
+- **SPDX-header CI guard.** `make spdx-check` (`scripts/spdx-check.sh`,
+  folded into `make check` and a dedicated CI job) fails the build if any
+  Go source — tracked or newly added — is missing its
+  `// SPDX-License-Identifier: GPL-3.0-or-later` header, keeping the 100%
+  coverage from regressing (ADR-0012).
+
 ## [1.2.1]
 
 ### Fixed

COMMITBRIEF.md

@@ -0,0 +1,171 @@
+# CommitBrief Review Rules
+
+You are a senior software engineer reviewing a diff before another human
+sees it. You wear two hats at once:
+
+1. **Adversarial defender.** Treat every line change as a potential
+   attack vector. Never assume input is sanitized, upstream checks
+   are sufficient, or the framework "handles it". If a code path
+   could be reached with adversarial input, surface the risk.
+2. **Optimization engineer.** Wasted CPU, memory, I/O, or money is a
+   real defect — it accumulates into latency, infra cost, and on-call
+   pages. Flag inefficiency the same way you flag bugs.
+
+Be precise, skeptical, and practical. Avoid vague advice ("be more
+careful"). Name specific functions, parameters, approaches.
+
+## What to look for
+
+### 1. Correctness
+- Logic errors, off-by-one, inverted conditionals, wrong default
+  branches.
+- Edge cases: nil / null / zero / negative / empty / boundary values;
+  integer overflow; type mismatches.
+- Concurrency: race conditions, deadlocks, unsafe shared state,
+  goroutine / thread leaks, missing cancellation propagation,
+  TOCTOU patterns.
+- Error handling: silently swallowed errors, wrong wrapping, lost
+  context, premature recovery that hides real failures.
+- Resource leaks: unclosed files, network connections, database
+  transactions, contexts, locks.
+- Wrong assumptions about idempotency, ordering, or atomicity.
+
+### 2. Security  (adversarial perspective — zero trust)
+
+Treat every addition as attack surface, even on internal services.
+
+- **Injection.** SQL, command/shell, NoSQL, LDAP, XSS, template
+  injection, log forging. Any user-controlled value reaching a
+  query / shell / template / log call is suspect until proven safe.
+- **Broken access control.** IDOR (object IDs that pass identity
+  checks but not ownership checks), missing or weakened
+  authorization, privilege escalation paths, exposed admin
+  endpoints, "trusted-internal" assumptions on listening services.
+- **Sensitive data exposure.** Hardcoded secrets (API keys, tokens,
+  passwords, private keys, JWT signing keys, DB credentials); PII
+  in logs or error messages; weak crypto (insecure algorithms,
+  hardcoded IVs, `math/rand` or equivalent for secrets); secrets
+  inadvertently serialized into JSON or stack traces.
+- **Security misconfiguration.** Debug or verbose modes left on,
+  default credentials, CORS too permissive, missing security
+  headers, overly broad file / database / cloud IAM permissions,
+  insecure deserialization formats.
+- **Race-condition risks with security impact.** Double-spend,
+  file-replace races, signal-handler races, atomic-counter
+  mistakes in auth flows.
+
+When you see what looks like a credential pattern (long random-
+looking string, `sk-`, `AKIA`, `-----BEGIN ... PRIVATE KEY-----`,
+etc.), flag it as **critical** even if you're unsure — false
+positives are cheap, leaked credentials aren't.
+
+### 3. Performance & efficiency
+
+Hot-path inefficiency is a defect; cold-path inefficiency rarely is.
+Distinguish. If you can't prove the access pattern is hot from the
+diff alone, label the finding **likely** and name what to measure
+(a benchmark, a profiler view, a specific metric).
+
+- **Algorithms & data structures.** Hidden O(n²) or worse (nested
+  scans, repeated linear search inside a loop), poor data-structure
+  choice (slice where a map would, list where a set would),
+  redundant sorts / filters / conversions, unnecessary copies /
+  serialization / parsing.
+- **Memory.** Allocations in tight loops, retained references /
+  leaks, unbounded cache or buffer growth, loading full datasets
+  where streaming or pagination would do.
+- **I/O & network.** Chatty calls (N small requests where one
+  batched call works), missing compression / keep-alive /
+  connection-pooling, blocking I/O in latency-sensitive paths,
+  redundant fetches of the same data (caching candidates).
+- **Database.** N+1 queries, `SELECT *` when columns suffice,
+  unbounded scans, missing index implied by the access shape,
+  inefficient join / filter / sort patterns, missing pagination
+  on potentially large result sets.
+- **Concurrency.** Serialized async work that could parallelize
+  safely, over-parallelization causing contention, lock contention
+  on a hot section, thread-blocking calls inside async code,
+  missing backpressure or queue size limits.
+- **Caching.** Obvious caches missing, wrong granularity (per-
+  request cache that should be per-process; per-process cache that
+  should be per-user), stale-invalidation strategy unclear,
+  cache-stampede risk on miss.
+- **Reliability / cost.** Infinite or unbounded retries without
+  jitter, polling loops where event-driven would do, redundant
+  LLM / API / billable-resource calls, timeouts too high (hangs)
+  or too low (cascading failures), rate-limit handling missing.
+
+### 4. Maintainability
+
+Structural debt that future-readers (or you in three months) will
+trip over.
+
+- Single-responsibility violations — functions or files doing too
+  many unrelated things.
+- Module boundary leaks — internal types escaping public APIs,
+  cross-package coupling that should go through an interface.
+- Abstraction level: under-abstracted (duplicated logic across N
+  call sites) vs. over-abstracted (one-use indirection layer hiding
+  intent without enabling reuse).
+- **Code reuse.** Repeated utility logic that should be extracted to
+  a shared helper. Similar queries / functions differing only by a
+  small parameter — candidates for parameterization.
+- **Dead code.** Unused functions / variables / imports / exports /
+  feature flags / config keys; deprecated paths still executed;
+  always-true / always-false branches; unreachable code after
+  return / throw / panic. When you find dead code, classify the
+  suggestion as one of: **safe to remove**, **needs verification
+  before removal** (might be used reflectively / via build tag /
+  by an external consumer), or **consolidate via a shared helper**
+  (live but duplicated).
+- Test coverage gaps for the behavior introduced.
+
+### 5. Code quality
+
+Surface-level issues that hurt readability and trust.
+
+- Naming clarity — variables that lie about their content, functions
+  whose name implies one job but does several.
+- Magic numbers and unexplained constants.
+- Non-idiomatic constructs for the language.
+- Comments that contradict the code below them.
+- Inconsistent error / log / null handling across the changed
+  surface.
+
+## What NOT to flag
+
+- Files filtered by `.commitbriefignore` or built-in defaults are
+  already excluded; do not ask why they're missing.
+- Pure formatting / whitespace unless it actively hurts readability.
+- Variable renames unless the new name clearly improves clarity.
+- Hypothetical future requirements — do not suggest a feature flag
+  or abstraction for something the diff doesn't ask for.
+- Do not repeat the diff back. Summarize and reference specific lines.
+- Do not invent file paths or line numbers. Reference only what
+  appears in the diff.
+
+## Output discipline
+
+Each finding's `description` is 1–3 sentences explaining the issue
+and its impact. Each `suggestion` is 2–3 sentences describing the
+concrete fix — name functions, parameters, approaches, not generic
+advice. Be specific and actionable.
+
+If you can't determine whether something is a real issue from the
+diff alone (e.g. you'd need to see how a function is called from
+unchanged code), prefer silence over speculation. False positives
+erode trust in the review faster than a few missed nits.
+
+## Project Context
+
+Edit the lines below for your project so the model picks up local
+conventions. Leaving them blank is fine; default reviews still work
+without them.
+
+- Language: Go
+
+<!--
+This file is your project's review rules. It is sent as a system
+prompt on every review. If it grows too large, run:
+  commitbrief compress
+-->

CONTRIBUTING.md

@@ -117,12 +117,46 @@ implementation that costs a few ns may still be net-positive).
 3. Material decisions or scope changes update the appropriate ADR
    (open a new ADR if needed — supersede rather than silently contradict
    an existing one).
-4. New dependencies pass `make license-check` (must be GPL-3.0-compatible —
-   MIT, Apache-2.0, BSD, ISC, MPL-2.0, GPL/LGPL-3.0+ are fine; AGPL and
-   proprietary are not).
+4. New dependencies follow the [Adding a dependency](#adding-a-dependency)
+   checklist and pass `make license-check`.
 5. Pre-release tags additionally pass `make release-check` (e.g. the
    `internal/rules/default.md` placeholder guard from PRD §10 / OQ-25).
 
+The fastest way to clear gates 1, 2, and 4 (plus the SPDX-header and
+i18n parity checks) in one shot is `make check` — the same gate CI runs.
+
+## Adding a dependency
+
+New third-party modules are accepted but deliberately rare. Every import
+becomes combined work under GPL-3.0 (the conservative reading for
+statically-linked Go), adds supply-chain surface, and grows the binary.
+Before adding one, work through this checklist:
+
+1. **Justify it.** Can the standard library or an already-required module
+   do the job? For a new LLM provider specifically, prefer reusing the
+   existing `openai-go` client against the provider's OpenAI-compatible
+   endpoint — the `deepseek` / `mistral` / `cohere` packages do exactly
+   this and add **zero** new dependencies — before pulling in a bespoke
+   SDK.
+2. **Check the license.** It must be on the GPL-3.0-compatible allow-list
+   maintained in `scripts/license-check.sh`: `Apache-2.0`, `BSD-2-Clause`,
+   `BSD-3-Clause`, `GPL-3.0(-or-later)`, `ISC`, `LGPL-3.0(-or-later)`,
+   `MIT`, `MPL-2.0`, `Unlicense`. **AGPL, proprietary, and anything not on
+   the list are rejected — no exceptions.** Check the module's own license
+   *and* its transitive dependencies.
+3. **Add and sync.** `go get <module>@<version>`, then `make tidy` so
+   `go.mod` / `go.sum` stay clean and minimal. Pin a specific released
+   version, not a branch or pseudo-version, unless there is no tagged
+   release.
+4. **Audit.** `make license-check` must pass locally — it runs
+   `go-licenses` over the entire module graph (transitive deps included),
+   not just your direct addition. CI enforces it too.
+5. **If it's incompatible, don't vendor it.** When a module you want ships
+   under an incompatible license, build the integration in-house or find a
+   compatible replacement rather than adding it.
+6. **Document.** State the new dependency and why in the PR description.
+   If it changes user-facing behavior, add a `CHANGELOG.md` entry.
+
 ## Cross-platform notes
 
 CommitBrief targets macOS, Linux, and Windows on amd64 and arm64 from a

Makefile

@@ -13,7 +13,7 @@ LDFLAGS := -s -w \
 
 GO ?= go
 
-.PHONY: help build test test-live bench lint fmt tidy clean check release-check license-check i18n-check security-check manpage smoke
+.PHONY: help build test test-live bench lint fmt tidy clean check release-check license-check i18n-check spdx-check security-check manpage smoke
 
 help: ## Show this help
 	@awk 'BEGIN {FS = ":.*## "} /^[a-zA-Z_-]+:.*## / {printf "  \033[36m%-18s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST)
@@ -57,10 +57,13 @@ license-check: ## Audit dependency licenses for GPL-3.0 compatibility
 i18n-check: ## Flag i18n catalog keys with no Go source reference (UC-25)
 	bash scripts/i18n-deadkey-check.sh
 
+spdx-check: ## Fail if any Go source file is missing the SPDX header (ADR-0012)
+	bash scripts/spdx-check.sh
+
 security-check: ## Run gosec with the documented exclusion set
 	bash scripts/security-scan.sh
 
-check: ## Run every guard CI runs (fmt-drift, vet, lint, test, release-check, i18n-check)
+check: ## Run every guard CI runs (fmt-drift, vet, lint, test, release-check, i18n-check, spdx-check)
 	@echo "==> gofmt drift"
 	@drift=$$(gofmt -l -s . | grep -v '^vendor/' || true); \
 	if [ -n "$$drift" ]; then \
@@ -76,6 +79,8 @@ check: ## Run every guard CI runs (fmt-drift, vet, lint, test, release-check, i1
 	@$(MAKE) -s release-check
 	@echo "==> i18n-check"
 	@$(MAKE) -s i18n-check
+	@echo "==> spdx-check"
+	@$(MAKE) -s spdx-check
 	@echo "==> security-check (gosec)"
 	@if command -v gosec >/dev/null 2>&1; then \
 		$(MAKE) -s security-check; \

README.md

@@ -28,8 +28,9 @@ read on your diff before another human (or your future self) sees it.
 - **Local-first.** Diffs and review output stay on your machine. The
   only network egress is to the provider you chose.
 - **Provider-agnostic.** Anthropic, OpenAI, Gemini, or Ollama as
-  API-backed providers; `claude-cli` and `gemini-cli` reuse your
-  local Claude Code / Gemini CLI subscription (no extra API key).
+  API-backed providers; `claude-cli`, `gemini-cli`, and `codex-cli`
+  reuse your local Claude Code / Gemini / Codex CLI subscription (no
+  extra API key).
 - **Cache aware.** Re-running on an unchanged diff is essentially free —
   one disk read, no token spend. `--verbose` shows what you saved.
 - **Custom review rules.** A repo's `COMMITBRIEF.md` is sent as the
@@ -183,7 +184,7 @@ with `--json`/`--markdown`/`--output`), `--compact`, `--no-cache`,
 (hide findings below this severity in the rendered output; `--json` and
 `--fail-on` still see the full set), `-f/--file` (repeatable),
 `-d/--dir` (repeatable), `--yes`, `--verbose`, `--quiet`, `--lang`,
-`--provider`, `--model`, `--cli <claude|gemini>` (shorthand for the
+`--provider`, `--model`, `--cli <claude|gemini|codex>` (shorthand for the
 CLI-tool-backed providers; mutually exclusive with `--json` /
 `--markdown`), `--allow-secrets` (acknowledge a flagged credential in
 the diff), `--no-cost-check` (skip cost preflight), `--color`. See
@@ -204,6 +205,7 @@ Four API providers + two CLI-tool-backed providers ship in the box:
 | **Ollama** | Whatever you've `ollama pull`'d | Local-only, no API key, no per-token cost. |
 | **`claude-cli`** | Whatever your local Claude Code uses | Subprocess of `claude -p -` — no API key on our side; reuses your Claude Code subscription. `commitbrief --cli claude --staged`. |
 | **`gemini-cli`** | Whatever your local Gemini CLI uses | Subprocess of `gemini -p` — no API key on our side; reuses your Gemini CLI auth. `commitbrief --cli gemini --staged`. |
+| **`codex-cli`** | Whatever your local Codex CLI uses | Subprocess of `codex exec --sandbox read-only --skip-git-repo-check` — no API key on our side; reuses your Codex CLI (ChatGPT) auth. `commitbrief --cli codex --staged`. |
 
 CLI-backed providers emit pre-formatted plain text — they bypass the
 structured-findings JSON path, the per-finding cards renderer, and the
@@ -219,8 +221,8 @@ subscription and don't want to manage a second API key.
 Adding a provider is one new package under `internal/provider/<name>/`.
 
 > The `remote pr` subcommand (below) requires an **API provider** —
-> `claude-cli` / `gemini-cli` are incompatible because they don't produce
-> structured findings.
+> `claude-cli` / `gemini-cli` / `codex-cli` are incompatible because they
+> don't produce structured findings.
 
 ## Reviewing pull requests from the terminal
 

cmd/commitbrief/main.go

@@ -13,6 +13,7 @@ import (
 	// a local subprocess rather than an HTTPS API.
 	_ "github.com/CommitBrief/commitbrief/internal/provider/anthropic"
 	_ "github.com/CommitBrief/commitbrief/internal/provider/claude-cli"
+	_ "github.com/CommitBrief/commitbrief/internal/provider/codex-cli"
 	_ "github.com/CommitBrief/commitbrief/internal/provider/cohere"
 	_ "github.com/CommitBrief/commitbrief/internal/provider/deepseek"
 	_ "github.com/CommitBrief/commitbrief/internal/provider/gemini"

internal/cli/remote.go

@@ -14,7 +14,8 @@ func newRemoteCmd() *cobra.Command {
 		Long: "Run CommitBrief against GitHub resources via your local `gh` CLI.\n" +
 			"Currently: `remote pr <ID>` reviews a pull request and posts findings\n" +
 			"as inline comments plus a review verdict. Requires an API provider\n" +
-			"(claude-cli / gemini-cli are incompatible — they don't produce findings).",
+			"(CLI-tool providers claude-cli / gemini-cli / codex-cli are incompatible —\n" +
+			"they don't produce structured findings).",
 	}
 	cmd.AddCommand(newRemotePRCmd())
 	return cmd

internal/cli/root.go

@@ -97,7 +97,7 @@ func newRootCmd() *cobra.Command {
 	flags.StringVar(&global.color, "color", "auto", "color output: auto, always, never")
 	flags.StringSliceVarP(&global.files, "file", "f", nil, "review only these files (repeatable); combines with the active scope flag")
 	flags.StringSliceVarP(&global.dirs, "dir", "d", nil, "review only files under these directories (repeatable); combines with the active scope flag")
-	flags.StringVar(&global.cli, "cli", "", "use a locally-installed CLI tool (claude|gemini) as the review backend; shorthand for --provider <name>-cli")
+	flags.StringVar(&global.cli, "cli", "", "use a locally-installed CLI tool (claude|gemini|codex) as the review backend; shorthand for --provider <name>-cli")
 	cmd.MarkFlagsMutuallyExclusive("provider", "cli")
 	// UC-07: CLI providers emit pre-formatted plain text that goes
 	// straight to the user. --json / --markdown drive structured

internal/i18n/messages.en.yml

@@ -127,7 +127,7 @@ doctor.detail.gitignore_no_entry: ".commitbrief/ not listed; run 'commitbrief se
 # remote pr — GitHub PR review via gh (ADR-0016, v1.1.0). Local stderr only;
 # text posted to GitHub stays hardcoded English in internal/remote.
 remote.gh_missing: "remote pr: the 'gh' CLI is required but not on PATH. Install it from https://cli.github.com (brew install gh / winget install GitHub.cli / dnf install gh)."
-remote.plain_text_provider: "remote pr: CLI-tool-backed providers (claude-cli / gemini-cli) can't produce structured findings. Use an API provider (anthropic / openai / gemini / ollama)."
+remote.plain_text_provider: "remote pr: CLI-tool-backed providers (claude-cli / gemini-cli / codex-cli) can't produce structured findings. Use an API provider (anthropic / openai / gemini / ollama)."
 remote.request_changes_on_info: "remote pr: --request-changes-on=info is not valid; you cannot request changes on an info-only review."
 remote.request_changes_on_invalid: "remote pr: invalid --request-changes-on=%q (use critical, high, medium, or low)."
 remote.self_pr_blocked: "remote pr: you are the author of this PR; GitHub does not allow self-review."