Review - Launch review agents. Each receives the PR diff, change summary, and relevant CLAUDE.md content. Each returns a list of issues with a brief description. Prefer running independent agents in parallel when possible.
Go Patterns & Architecture (godev agent): Component registration (single vs batch MustRegister*), ConfigSpec construction, field name constants, ParsedConfig extraction, Resources pattern, import organization, license headers, formatting/linting, error handling (wrapping with gerund form, %w), context propagation (no context.Background() in methods, no storing ctx on structs), concurrency patterns (mutex, goroutine lifecycle), shutdown/cleanup (idempotent Close, sync.Once), public wrappers, bundle registration, info.csv metadata, distribution classification. Also flag CONTRIBUTING.md §3.1.3 violations: connector code that is a line-by-line port of an implementation from another ecosystem (e.g. Debezium for CDC) — carrying over that source's abstractions, idioms, or naming rather than being written as idiomatic Go. The connector should be designed to support its endpoint well per the rest of CONTRIBUTING.md (UX, config knobs, observability, reliability), not for parity with the reference tool. This is only a finding when the port is clear (e.g. ported comments, foreign naming conventions, mirrored class hierarchies), not for any code that merely resembles a known design.
Tests (tester agent): Unit: table-driven tests with errContains, assert vs require, config parsing with MockResources, enterprise InjectTestService, processor/input/output/bloblang lifecycle tests, config linting, NewStreamBuilder pipelines, HTTP mock servers. Integration: integration.CheckSkip(t), Given-When-Then with t.Log(), testcontainers-go (module helpers preferred, GenericContainer fallback), NewStreamBuilder with AddBatchConsumerFunc, side-effect imports, async stream.Run with context.Canceled handling, assert.Eventually polling (no require inside), parallel subtest safety, cleanup with context.Background(). Flag changed code lacking tests and new components without integration tests.
Bugs and Security (general-purpose agent): Logic errors, nil dereferences, race conditions, resource leaks, SQL/command injection, XSS, hardcoded secrets. Focus on real bugs, not nitpicks.
Benchmarking (general-purpose agent): Only run this agent if the PR touches files under internal/impl/*/bench/ or adds/modifies a connector's performance-critical path. Checks:
- If the PR adds or modifies a
bench/ directory, verify it includes a README.md with prerequisites, how-to-run, and expected output sections.
- If the PR includes benchmark results (throughput numbers in the PR description), verify the corresponding results file in
docs/benchmark-results/ is updated. Flag if results are only in the PR description but not recorded in the results file.
- If the PR adds a new benchmark suite, verify it follows the structure in
docs/benchmarking.md: Taskfile.yaml, benchmark_config.yaml, data generation scripts, and README.md.
- If the PR modifies a connector in a way that could affect throughput (e.g. changes to batching, buffering, connection handling, serialization), note that a benchmark re-run may be warranted and check whether
docs/benchmark-results/ was updated.
- Verify the non-engineering summary in
docs/benchmark-results/SUMMARY.md is updated if new connectors are benchmarked or if throughput numbers change significantly.
Certification & Contribution Guidelines (general-purpose agent): Audits the PR against the entire CONTRIBUTING.md (the connector certification & contribution spec; applies to internal and external contributions). Its job is to ensure no part of the document is missed. For areas owned by another agent, DEFER to that agent and do not duplicate its findings — cover the gaps and confirm certification-level expectations are met. Cite the exact clause (e.g. "§1.2.2") in every finding. Walk the document section by section:
- §1.1 Documentation & UX — For new or changed connectors, verify user-facing docs exist and are adequate: component
Summary()/Description(), per-field documentation in the ConfigSpec, config examples, and any docs/modules/... pages. Flag a new connector or field with no description or examples (§1.1.1), or missing usage/pitfall/troubleshooting docs (§1.1.2). Do not flag subjective wording or polish.
- §1.2 Observability & Debuggability — Bounded-cardinality metrics (§1.2.1); logging discipline — nothing on the happy path, warn/error on unexpected (§1.2.2); documented limitations (§1.2.3); strong config validation/linting (§1.2.4). Coordinate with
godev. Flag a new connector that emits no metrics, logs during normal operation, or omits config validation.
- §1.3 Reliability & Testing — Idiomatic Go → defer to
godev (§1.3.1); e2e/cross-config + CI integration tests → defer to tester (§1.3.2–1.3.3); benchmarking → defer to the Benchmarking agent (§1.3.4–1.3.6). Here, only confirm the certification-level expectation: a new certified connector ships integration tests and a benchmark.
- §2 Connector Selection Criteria — Triage-level, not a code property. Raise only as a NOTE (never blocking) when a PR introduces a brand-new connector for a niche/declining (§2.2.1) or hard-to-test (§2.2.2) technology, so a maintainer can confirm it was scoped (ties to §3.1.1).
- §3.1 Required Engineering Qualities — Community contributions are pre-scoped/authored or tied to a GitHub issue (§3.1.1); Go-first / no foreign port → defer to
godev (§3.1.3); live credential rotation where applicable (§3.1.7).
- §3.2 Anti-Patterns — Overlaps
godev and the Bugs/Security agent; defer to them.
- §3.3 Change Size — Defer to the Commit Policy agent.
- §4 Client Library Evaluation — When the PR adds a new third-party client for the target system (new dependency in
go.mod), check §4.1/§4.2: actively maintained, well-adopted in Go, semver ≥ v1, no known-abandoned or insecure library. Flag a new connector built on an unmaintained or pre-v1 client (note the library and the evidence).
- §5 CDC Connector Standard — defer to the CDC Connector Standard agent below.
CDC Connector Standard (general-purpose agent): Runs only when the PR touches a CDC connector. Detect that via a changed file under a directory whose package registers a *_cdc/*_changefeed input, OR a new such registration in the diff. Do not rely on the registration being in the diff — for an edit to an existing connector, derive the fleet by grepping internal/impl for (Must)?Register(Batch)?Input names matching /(_cdc|_changefeed)$/ (registrations may be multi-line) and intersect with the changed connector directories. The rubric is conformance to the existing fleet, read from code: first Read the canonical references named in CONTRIBUTING §5 (internal/impl/oracledb, internal/impl/mysql, internal/impl/postgresql) and compare against them. Do NOT rely on external knowledge of formats like Debezium — anchor every finding to an in-repo file you have read, and cite the §5 clause.
- Scope the enforcement to the change. New CDC connector (new registration) → enforce all of §5. Modifying an existing connector → enforce only the §5 clauses whose surface the PR actually touches; do NOT flag pre-existing gaps unrelated to the diff (those are tracked separately), and a doc-only/unrelated change triggers nothing here.
- §5.1.1 naming — the input is registered
*_cdc/*_changefeed.
- §5.2 message shape — flat top-level metadata + raw body; flag a nested
before/after/source/op envelope, or component-prefixed metadata keys (e.g. sap_hana_table vs the fleet's table), citing the canonical connector's actual keys.
- §5.3 config names — flag bespoke names where a canonical one exists (
enable_snapshot→stream_snapshot, cursor_cache→checkpoint_cache, checkpoint_key→checkpoint_cache_key, snapshot_batch_size→snapshot_max_batch_size). The deterministic gate is the conformance test internal/plugins/cdctest — a new connector must pass it strictly.
- §5.4.1 checkpoint — a
checkpoint_cache field is present and the checkpoint is saved at snapshot completion (before the stream loop); flag in-memory-only cursors or checkpoint-only-after-first-stream-message.
- §5.4.6 decimals —
sqlutil.CanonicaliseDecimal on the numeric path; flag float64 for DECIMAL/NUMERIC.
- §5.4.3 / §5.4.4 / §5.5.1 — note absent
ConnectionTest, tracing spans, or schema (PK + nullability) metadata when the PR adds or changes the relevant surface.
- For §5.2's "not a port" rule, the operational check is divergence from the fleet's flat shape — do not attempt to judge Debezium-likeness in the abstract (defer the foreign-port judgment to
godev/§3.1.3).
Commit Policy (general-purpose agent): Uses gh pr view --json commits on the PR commits. Checks:
- Granularity: Each commit is one small, self-contained, logical change. Flag commits mixing unrelated work. In multi-commit PRs, documentation changes must be in a separate commit from code changes.
- Change size (CONTRIBUTING.md §3.3): The ~10K-line limit counts code a reviewer must actually read — including AI-generated code — and exists to keep changes reviewable. Estimate reviewable code lines from
gh pr view --json additions,deletions and git show --stat, then exclude only what isn't read line-by-line: mechanically generated/derived files (codegen output, mocks, bundle imports), vendored code, lockfiles, and non-code (docs/*.md, .claude/ skills, Terraform *.tf/*.tftpl/*.hcl, *.tmpl templates, testdata/fixtures). Flag when the remaining reviewable code approaches ~10K lines, or when a very large code change lands with no explanation. Do NOT flag a PR whose size is dominated by non-code or generated files (e.g. a skills/docs/infra PR). When flagging, recommend splitting into smaller, independently reviewable PRs (§3.3.2).
- Message format (enforced): Must match one of these patterns:
system: message — lowercase system name matching a known area (e.g., otlp: add authz support, kafka: fix consumer group rebalance)system(subsystem): message — same, with parenthesized subsystem (e.g., gateway(authz): add http middleware, cli(mcp): handle shutdown)chore: message — low-importance cleanup, maintenance, or housekeeping changes (e.g., chore: update gitignore)- Sentence-case plain message for repo-wide changes not scoped to one system (e.g.,
Bump to Go 1.26, Update CI workflows). First word capitalized, rest lowercase unless proper noun.
Revert "..." and merge commits are exempt.
In all cases, message starts lowercase and uses imperative mood (e.g., "add", "fix", not "added", "fixes").
- Message quality (enforced): Flag messages that are vague ("fix stuff", "updates", "WIP"), misleading (title doesn't match the actual changes), or incomprehensible.
- Fixup/squash: Flag unsquashed
fixup!/squash! commits.
- Ignore PR number suffixes
(#1234).
