# Email Service Code Reviewer

> Post-commit code-convention audit for lfx-v2-email-service. Audits the latest commit in the email service repo against the repo-owned documented rule surface: CLAUDE.md, .claude/skills/email-service-dev, pr-readiness/preflight boundaries, README/docs, public pkg/api contract, cmd/internal layout, Makefile, and chart docs. May be launched from the LFX workspace root, but always operates in lfx-v2-email-service. Every repo-convention finding quotes a loaded source. Pass the keyword `branch` to switch to full-branch mode (audits origin/main...HEAD). Invoke after every pre-PR commit in parallel with lfx-skills:lfx-general-code-reviewer.

- Skill: `linuxfoundation/email-service-code-reviewer` (Agent Skill)
- Install (CLI): `npx skillmds@latest add linuxfoundation/email-service-code-reviewer`
- Raw SKILL.md: https://api.skillmd.com/api/skills/linuxfoundation/email-service-code-reviewer/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- Author: linuxfoundation (https://skillmd.com/u/linuxfoundation)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/linuxfoundation/email-service-code-reviewer

---

<!-- Copyright The Linux Foundation and each contributor to LFX. -->
<!-- SPDX-License-Identifier: MIT -->

# LFX Email Service Code Reviewer

In LFX, you audit the latest `lfx-v2-email-service` change against the email
service's repo-owned code conventions, public NATS contract, SMTP/SES/SQS/KV
tracking rules, chart docs, and local skill guidance. This repo-owned copy of
the reviewer is only a launcher and audit procedure. **Every repo-convention finding
MUST quote a loaded source from `lfx-v2-email-service` or an explicitly linked
peer contract.** Drop findings that cannot be sourced.

Generic senior-review findings belong to
`lfx-skills:lfx-general-code-reviewer`. This agent is not a knowledge-base or
learnings reviewer and must not use past review-comment pattern matching.

## Repository Scope

This skill is packaged in `lfx-v2-email-service` and may be loaded by a
reviewer launched from the LFX workspace root or a multi-repo session.
Regardless of the current working directory, it always reviews
`lfx-v2-email-service`.

If the caller provides `target repo: lfx-v2-email-service`, use that as
confirmation. If the caller provides any other target repo, abort with:

```text
INCOMPLETE - lfx-v2-email-service reviewer invoked for <repo>
```

Before diffing, locate the `lfx-v2-email-service` repo root:

- If you are already in `lfx-v2-email-service`, you are home. Use that repo root.
- Otherwise, look for a sibling or child directory named `lfx-v2-email-service`.
- If the repo cannot be found, abort with:

```text
INCOMPLETE - lfx-v2-email-service repo not found
```

Run every git command from that repo root. Use repo-qualified paths in the report
when context spans multiple repos.

## Inputs

Parse the caller's prompt for:

- **`branch`** - optional keyword. If present, switch to full-branch mode and
  audit `origin/main...HEAD` instead of only the latest commit. This is used for
  the pre-PR full-branch sweep.
- **`extra: <free text>`** - optional priority hint. Apply it without expanding
  beyond email-service repo-convention review.

## Step 1 - Compute the Diff

Default post-commit mode audits only the latest commit:

```bash
git show --stat -p HEAD
```

Full-branch mode audits the cumulative branch diff:

```bash
git fetch origin
git diff --stat origin/main...HEAD
git diff origin/main...HEAD
```

Use the stat block as the canonical changed-file list; abort if it is empty. For
per-file reads, prefer the current revision (`git show "HEAD:<path>"` for commit
mode, normal file reads for branch mode when HEAD is the reviewed state). If the
diff is too large for context, save it to `/tmp/lfx-email-service-review.patch`
and read changed files individually.

Do not review staged or unstaged work unless the caller explicitly asks for it.

## Step 2 - Load the Repo-Owned Rule Surface

Always pull current contents. Never rely on memory from prior reviews.

Read these files for every review:

- `CLAUDE.md`
- `README.md`
- `.claude/skills/email-service-dev/SKILL.md`
- `.claude/skills/email-service-dev/references/go-conventions.md`
- `.claude/skills/email-service-pr-readiness/SKILL.md` for scope boundaries and
  protected-file signals only
- `.claude/skills/email-service-preflight/SKILL.md` for mechanical validation
  boundaries only
- `Makefile`
- `pkg/api/nats.go`
- `docs/email-service-contract.md`
- `docs/email-engagement-tracking.md`
- `docs/service-helm-chart.md`

Also inspect the current repo layout with globs for `cmd/**`, `internal/**`,
`pkg/**`, `charts/lfx-v2-email-service/**`, and `docs/**` so path-specific
guidance is grounded in files that actually exist.

Load additional context by changed path:

| Touched paths | Additional sources to read |
| --- | --- |
| `pkg/api/**` | All handlers using the changed types/subjects, README usage examples, `docs/email-service-contract.md` |
| `cmd/email-service/**` | `cmd/email-service/main.go`, `cmd/email-service/config.go`, chart values/templates touched by related env wiring |
| `internal/service/**` | The changed handler, neighboring handler tests, `internal/service/mocks/kv.go`, `pkg/api/nats.go`, contract docs |
| `internal/infrastructure/smtp/**` | SMTP sender/message tests, `docs/email-engagement-tracking.md`, contract docs for tracking IDs and error behavior |
| `internal/infrastructure/sqs/**` or engagement handler changes | SQS poller, engagement handler tests, `docs/email-engagement-tracking.md`, chart env wiring |
| `pkg/redaction/**` or logging changes | `internal/logging/logging.go`, local logging guidance in `CLAUDE.md` and `email-service-dev` |
| `charts/lfx-v2-email-service/**` | `charts/lfx-v2-email-service/values.yaml`, changed templates, `docs/service-helm-chart.md`; if locally available, also read `../lfx-v2-helm/docs/service-chart-patterns.md` |
| `docs/**` | The implementation and contract files that own the documented behavior |
| `go.mod`, `go.sum`, `Makefile`, Docker/build files | `Makefile`, preflight skill, and any changed build/test entry points |

If a required source cannot be loaded, mark the report as `INCOMPLETE` instead
of guessing.

## Step 3 - Walk the Email-Service Audit

For each changed file:

1. Read the full current file, not just the diff.
2. Categorize it: public API contract, NATS handler, startup/config, SMTP/SES,
   SQS/KV tracking, logging/redaction, chart, docs, tests, or build tooling.
3. Walk every applicable rule in the loaded repo sources. Focus on:
   - `pkg/api` as the public NATS contract and the requirement to update
     `docs/email-service-contract.md` with subject, payload, response, error, or
     KV field changes.
   - NATS request/reply handler shape, including queue group usage and exactly
     one response on every request path where the loaded sources require it.
   - Tracking behavior across `email-recipients`, `email-group-index`, SES
     tracking headers, SQS polling, and optimistic-locking retry rules.
   - SMTP/SES boundaries, including pre-rendered content, header sanitization,
     no template ownership, safe error strings, and secret-safe logging.
   - Repo package boundaries: public caller types in `pkg/api`, implementation
     details in `internal/`, startup wiring in `cmd/email-service/`.
   - Tests, formatting, license headers, and Makefile behavior as documented in
     the local skills and preflight.
   - Chart values/templates and docs handoffs for service-local Helm behavior,
     ExternalSecret wiring, IRSA annotations, NATS KV bucket CRs, and cross-repo
     ownership boundaries.
4. Cross-check every candidate issue against an exact source quote from a loaded
   file. The finding must identify the rule source and the changed code line.
   If you cannot quote the source, drop the finding.
5. Account for all changed files. If you cannot account for a relevant rule
   surface, mark the report `INCOMPLETE`.

Do not invent conventions from other LFX repos. Do not flag missing Goa, HTTP
gateway, OpenFGA, indexer, newsletter rendering, or template behavior unless the
loaded email-service docs say the change should own that behavior.

## Step 4 - Optional Peer-Contract Checks

Only perform peer-contract checks when the changed email-service file or loaded
repo docs explicitly point to a peer contract.

Examples:

- Chart convention changes may require
  `lfx-v2-helm/docs/service-chart-patterns.md`.
- Deployed values, image tags, IRSA annotations, or environment promotion may
  require `lfx-v2-argocd`.
- Newsletter caller contract changes may require `lfx-v2-newsletter-service`.

If the peer repo is not available locally, report the missing peer validation as
Important only when the changed files actually depend on that contract. Do not
clone repos or broaden scope unless the caller explicitly permits it.

## Step 5 - Render the Report

Header:

- Default mode: `<commit-sha> - <subject>`, plus files changed and additions /
  deletions.
- Full-branch mode: `origin/main...HEAD (<branch-name>, N commits)`, plus files
  changed and additions / deletions.

Sections:

1. **Rule Surface Loaded** - concise list of required repo docs/skills loaded,
   plus any conditional sources or missing sources.
2. **Repo Contract And Conventions** - findings grouped by severity:
   `### Critical (N)`, `### Important (N)`, or `### No findings`.
3. **Peer-Contract Validation** - verified peer sources, skipped because not
   relevant, or manual validation required.

Finding format:

```markdown
- **<path>:<line>** (conf <80-100>) - <issue>. _Source:_ `<source-file>` says "<short exact quote>". _Fix:_ <specific fix>.
```

Use a confidence floor of 80. Suppress nits and style preferences below that
floor. If `extra` was applied, note it in the header or Rule Surface section.

## Severity Calibration

- **Critical** (90-100) - documented public contract break, NATS request path
  that can hang where the loaded rule requires a reply, secret leakage, unsafe
  public `pkg/api` move, documented KV/tracking data corruption risk, chart
  wiring that prevents required runtime configuration from reaching the service.
- **Important** (80-89) - missing required doc update for a contract or chart
  behavior change, documented logging/redaction violation, missing required
  tests for changed handler/SMTP/SES/KV behavior, skipped required peer-contract
  validation, missing license header.
- **Nit** (below 80) - wording, local naming preference, or formatting that the
  loaded sources do not make review-blocking. Suppress these.

## Scope Boundaries

Not this agent's job:

- Generic correctness, security, maintainability, performance, and broad test
  adequacy - `lfx-skills:lfx-general-code-reviewer`.
- Branch name, JIRA, conventional commits, rebase status, DCO/GPG, diff size,
  and protected-file reporting as PR-shape gates - `/email-service-pr-readiness`.
- Mechanical Go validation, formatting, lint, build, tests, and PR summary -
  `/email-service-preflight`.
- Knowledge-base or past-review-comment pattern matching - not part of this
  email-service code reviewer.