# Lfx Self Serve Code Review

> Post-commit code-convention audit for lfx-self-serve. Audits the latest commit in the lfx-self-serve repo against the repo documented rule surface: `.claude/rules/`, the four `docs/reviews/` checklists, architecture docs, and upstream API contracts. May be launched from the LFX workspace root, but always operates in `lfx-self-serve`. Every repo-convention finding quotes a loaded source. Pass the keyword `branch` to switch to full-branch mode (audits the branch's diff against main — used for the pre-PR full-branch sweep). Renders a markdown review with Upstream API / data-layer validation and Repo conventions sections. Invoke after every commit while pre-PR, in parallel with `lfx-skills:lfx-general-code-review` and `lfx-self-serve-learnings-review`.

- Skill: `linuxfoundation/lfx-self-serve-code-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add linuxfoundation/lfx-self-serve-code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/linuxfoundation/lfx-self-serve-code-review/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/lfx-self-serve-code-review

---


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

# LFX Self-Serve Code Reviewer

In LFX, you audit the latest commit on the LFX Self-Serve branch against the repo's documented rule surface and upstream API/data-layer contracts. Load this repo's documented rules (`.claude/rules/`), the four `docs/reviews/` checklists, architecture docs, then walk them with cross-check discipline. **Every repo-convention finding MUST quote a loaded source** — drop unsourced claims to avoid inventing repo conventions.

Generic senior-review findings belong to `lfx-skills:lfx-general-code-review`. Empirical-pattern matches against past PR review comments belong to `lfx-self-serve-learnings-review`, not here.

## Repository scope

This skill is owned by the `lfx-self-serve` repo
(`.claude/skills/lfx-self-serve-code-review/`), and the reviewer loading it may
be launched from the LFX workspace root or a multi-repo session. Regardless of
the current working directory, it always reviews `lfx-self-serve`.

If the caller provides `target repo: lfx-self-serve`, use that as confirmation.
If the caller provides any other target repo, abort with
`INCOMPLETE - lfx-self-serve reviewer invoked for <repo>`.

Before diffing, locate the `lfx-self-serve` repo root:

- If you are already in `lfx-self-serve`, you are home. Use that repo root.
- Otherwise, look for a sibling or child directory named `lfx-self-serve`.
- If the repo cannot be found, abort with
  `INCOMPLETE - lfx-self-serve repo not found`.

## Inputs

Parse the caller's prompt for:

- **`branch`** — OPTIONAL keyword. If present, switch to full-branch mode: audit the branch's diff against main (`origin/main...HEAD`) instead of just the latest commit. Used by the pre-PR full-branch sweep.
- **`extra: <free text>`** — optional priority hint.

## Step 1 — Compute the diff

Run all git commands from the `lfx-self-serve` repo root.

Default mode: `git show --stat -p HEAD` — audits only the latest commit (not staged / unstaged work). Use the stat block as the canonical changed-file list; abort if empty.

Full-branch mode (`branch` passed): `git fetch origin && git diff --stat origin/main...HEAD && git diff origin/main...HEAD` — the branch's diff against main, i.e., everything HEAD adds vs `origin/main`.

For per-file reads: `git show "HEAD:<path>"`. If the diff is too big for context, save to `/tmp/code-review-diff.patch` and Read changed files individually.

Commit-level data is not your concern — signatures are checked by `/lfx-self-serve-pr-readiness` pre-PR; prior review comments are verified by `/lfx-review-pr` post-PR.

## Step 2 — Convention audit

Now layer the LFX-specific conventions on top: load the docs, then walk the audit using them. Step 2.1 sets up the source-of-truth surface; Step 2.2 audits against it with cross-check discipline.

### 2.1 — Load reference documents

Always pull current contents — never rely on memory of these files from prior runs.

**Always read (in parallel):**

- `CLAUDE.md` at the repo root
- `~/.claude/CLAUDE.md` if it exists
- Every file matching `.claude/rules/*.md` — Glob dynamically; never hand-maintain a list

**⚠ Mandatory: the four `docs/reviews/` checklists.** These are the **primary audit surface** — each item exists because it broke a real PR on this repo. Skipping a relevant checklist invalidates the audit. If you cannot Read a required checklist, mark the report as **INCOMPLETE**.

| Touched paths                         | Required checklist                         |
| ------------------------------------- | ------------------------------------------ |
| `apps/lfx-one/src/app/**`             | `docs/reviews/frontend-checklist.md`       |
| `apps/lfx-one/src/server/**`          | `docs/reviews/backend-checklist.md`        |
| `packages/shared/**` or Snowflake SQL | `docs/reviews/shared-and-sql-checklist.md` |
| `docs/**`                             | `docs/reviews/docs-checklist.md`           |

You audit **by these checklists**, not against your general framework knowledge. Locate a specific checklist item, rule file entry, or architecture-doc paragraph for every Repo-conventions finding (see 2.2).

**Architecture docs — load conditionally by changed-file paths:**

| Touched paths                                  | Load                                                                                                                  |
| ---------------------------------------------- | --------------------------------------------------------------------------------------------------------------------- |
| `apps/lfx-one/src/app/**`                      | `docs/architecture/frontend/angular-patterns.md`, `component-architecture.md`, `styling-system.md`                    |
| Drawer component or `DialogService.open` usage | `docs/architecture/frontend/drawer-pattern.md`                                                                        |
| `apps/lfx-one/src/server/**`                   | `docs/architecture/backend/README.md`, `error-handling-architecture.md`, `logging-monitoring.md`, `server-helpers.md` |
| `middleware/auth*`                             | `docs/architecture/backend/authentication.md`                                                                         |
| `auth-helper`, persona helpers                 | `docs/architecture/backend/impersonation.md`                                                                          |
| `/public/**` routes, public meetings           | `docs/architecture/backend/public-meetings.md`                                                                        |
| Pagination helpers, list endpoints             | `docs/architecture/backend/pagination.md`                                                                             |
| `ai.service.ts`, AI proxy calls                | `docs/architecture/backend/ai-service.md`                                                                             |
| `nats.service.ts`, project NATS RPCs           | `docs/architecture/backend/nats-integration.md`                                                                       |
| `snowflake.service.ts`, direct SQL             | `docs/architecture/backend/snowflake-integration.md`                                                                  |
| SSR / `server.ts` / render pipeline            | `docs/architecture/backend/ssr-server.md`                                                                             |
| `packages/shared/**`                           | `docs/architecture/shared/package-architecture.md`                                                                    |
| `*.spec.ts` or `e2e/**`                        | `docs/architecture/testing/e2e-testing.md`, `docs/architecture/testing/testing-best-practices.md`                     |

If `.claude/skills/self-serve-dev/references/` exists, also Read the relevant reference files (`backend-endpoint.md` for server changes, `frontend-code-generation.md` for frontend changes, etc.).

### 2.2 — Walk the audit

For each changed file:

1. **Read the full file at the current revision** — don't audit from diff alone; context matters.
2. **Categorize** — frontend component / frontend service / SSR / backend service / controller / route / shared / SQL / docs / mixed.
3. **Walk every applicable item** in the relevant `docs/reviews/` checklist + every applicable rule in `.claude/rules/` + the architecture docs loaded in 2.1.
4. **Cross-check before emitting:** for each candidate finding, locate the exact rule, checklist item, or architecture-doc paragraph it violates. Quote that source in the finding's `_Source:_` citation. **If you cannot quote the source, drop the finding** — hallucinated rules are worse than missed ones. These findings go in the **Repo conventions** section of the Step 4 report.
5. **Account for the full checklist surface:** if you cannot account for having considered every applicable checklist item, mark the report **INCOMPLETE** in Step 4 rather than ship a partial report.

## Step 3 — Upstream API / data-layer contract validation (backend only)

**Skip this entirely if no files under `apps/lfx-one/src/server/` were changed.**

The LFX One backend is a thin proxy to external Go microservices. New or modified proxy endpoints must align with the upstream API contract.

Required for:

- Any new file in `services/`, `controllers/`, or `routes/`
- Any modified service that calls `MicroserviceProxyService.proxyRequest()`
- Any new API path or changed request/response shape

For modifications to existing endpoints (not new ones), a lighter check is acceptable — verify the endpoint still exists rather than a full schema comparison.

```bash
gh api repos/linuxfoundation/<repo-name>/contents/gen/http/openapi3.yaml --jq '.content' | base64 -d
gh api repos/linuxfoundation/<repo-name>/contents/design --jq '.[].name'
gh api repos/linuxfoundation/<repo-name>/contents/design/<file>.go --jq '.content' | base64 -d
```

Upstream repo map:

| Domain        | Repo                          |
| ------------- | ----------------------------- |
| Queries       | `lfx-v2-query-service`        |
| Projects      | `lfx-v2-project-service`      |
| Meetings      | `lfx-v2-meeting-service`      |
| Mailing Lists | `lfx-v2-mailing-list-service` |
| Committees    | `lfx-v2-committee-service`    |
| Voting        | `lfx-v2-voting-service`       |
| Surveys       | `lfx-v2-survey-service`       |

Validate:

- Endpoint paths and HTTP methods match upstream
- Request body / query params match upstream schema (no extra fields, no missing required)
- Response shape matches upstream
- Query Service conventions: `page_size` (NOT `limit`), `page_token` for cursor pagination, `filters` format `field:value`
- No fabricated endpoints — if upstream doesn't expose it, the proxy shouldn't pretend it exists

**Snowflake direct SQL:** every `?` placeholder must have a corresponding value in the binds array, in the correct order. Bind mismatch is always Critical.

**On `gh api` failure** (404, auth, network): surface in Step 4's "Upstream API / data-layer validation" section as "Upstream API contract for `<service>` could not be verified — manual validation required." Treat as Important severity. Don't silently skip.

## Step 4 — Render the report

Header: `<commit-sha> — <subject>` (default) or `origin/main...HEAD (<branch-name>, N commits)` (full-branch mode), plus files changed and additions / deletions.

Two sections, in order. Each findings section groups under `### Critical (N)` (conf 90-100) and `### Important (N)` (conf 80-89), with `### No findings` if none clear the ≥80 floor.

1. **Upstream API / data-layer validation** (Step 3): verified paths, SQL bind checks, "manual validation required" flags, or "Skipped — no backend changes".
2. **Repo conventions** (Step 2.2): `- **<file>:<line>** (conf <0-100>) — <issue>. _Source:_ <quoted rule citation>. _Fix:_ <suggestion>.`

If a required checklist or architecture doc couldn't be loaded, lead with `INCOMPLETE — couldn't load <file>`. If `extra` was applied, note it.

## Severity calibration

Repo-convention and upstream-validation findings share the same buckets and ≥80 floor. Examples illustrative, not exhaustive.

- **Critical** (90-100) — SQL bind mismatches, upstream contract violations, bypassed user authorization, M2M misuse, raw `new Error()` / manual `res.status().json()`.
- **Important** (80-89) — documented style / structure violations (logger usage, license headers, PrimeNG wrappers, `@if`/`@for`, `inject()` DI, `page_size` over `limit`), unverified upstream contracts.
- **Nit** (below 80) — preferences, minor improvements, file naming. Suppressed by the ≥80 floor, except `category: upstream-api` deterministic flags.

## Known false positives — DO NOT emit

- Missing `ChangeDetectionStrategy.OnPush` — the app uses stable zoneless change detection.
- Missing `standalone: true` — Angular 20+ defaults to standalone.
- `provideZonelessChangeDetection()` flagged as experimental — it is stable in Angular 20.
- Any Repo conventions finding whose `_Source:_` citation cannot be quoted from a loaded rule file / checklist / architecture doc — drop.

## Scope boundaries — NOT this agent's job

- **PR-shape sanity** (branch name, JIRA, conventional commits, rebase, DCO + GPG, diff size, **protected files touched**) → `/lfx-self-serve-pr-readiness`.
- **Generic senior code review** (correctness, security, maintainability, performance, tests, code truthfulness) → `lfx-skills:lfx-general-code-review`.
- **Empirical pattern matching** (KB of past-PR review comments) → `lfx-self-serve-learnings-review`.