# Hive Elixir Review

> Project-specific PR-review rules for the tuist/hive Phoenix/Elixir codebase. Focuses on open-source secret hygiene, Noora data-part styling, OIDC auth, Helm deployment boundaries, release conventions, and a small set of Phoenix/Ecto/LiveView review rules that require diff awareness, semantic context, or cross-file reasoning that `mix format`/`mix credo` cannot do.

- Skill: `tuist/hive-elixir-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add tuist/hive-elixir-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/tuist/hive-elixir-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: DevOps & Infra
- Author: tuist (https://skillmd.com/u/tuist)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/tuist/hive-elixir-review

---


# Hive Elixir Review

This skill is intentionally narrow. Generic Elixir style, naming,
formatting, pipe chains, and lint hygiene are covered by `mix format`
and `mix credo` (including the custom checks under
`credo/checks/`). **Do not flag anything credo already catches** —
in particular: `Repo.*` calls inside `Enum.*` / `Stream.*` / `for`
(N+1 shape), or `timestamps/1` without an explicit `type:`. Focus on the
rules below because they need diff awareness, semantic context, or
cross-file reasoning that credo cannot do.

For each finding, cite `path:line` (or `Module.function/arity`) and
quote the relevant snippet.

Only report findings whose cited snippet is present in the PR diff. If
the concern comes from unchanged context, do not emit a finding, do not
mention it as a note, and do not create a "findings outside this PR's
diff" section. If every possible concern is outside the diff, return no
findings.

Do not infer violations from nearby lines. A finding must be anchored on
a changed line in the diff.

---

## 1. Open-source secret hygiene

Hive is an open-source repository. Production secrets live in the
`hive-k8s-production` 1Password vault and are referenced from source only
by name.

### Flag

- A real or plausible credential, token, OAuth secret, kubeconfig,
  database URL, Docker config JSON, service account token, `.env`
  contents, or private key added to source, tests, fixtures, docs,
  workflows, Helm values, or commit-oriented metadata. **Severity:
  critical.**
- Test fixtures or example env values that look real instead of obvious
  placeholders such as `"client-id"` or `"google-client-secret"`.
  **Severity: high.**
- Production `ExternalSecret` or Helm values that inline secret values
  instead of referencing remote 1Password item names and fields.
  **Severity: critical.**

### Do not flag

- Existing low-stakes secret reference names such as
  `hive-google-oauth/credential`, `hive-secret-key-base/password`, or the
  vault name `hive-k8s-production`.
- Phoenix development `secret_key_base` values in `config/dev.exs` or
  test-only placeholder secrets.

---

## 2. Styling with Noora and `data-part`

Hive styles server-rendered Phoenix HTML with Noora. The CSS convention
is one anchor selector per layout, component, or route, with internal
regions selected via `data-part`.

### Flag

- New CSS that introduces BEM-style child selectors such as
  `.headerbar__left`, `.layout__main`, or `.account-dropdown__menu`.
  Use nested `[data-part="..."]` selectors under the anchor instead.
  **Severity: medium.**
- New route-specific CSS that is not scoped under the route root id, for
  example `#login` or a future route id. **Severity: medium.**
- New component or layout CSS that reaches for utility or atomic classes
  instead of a component/layout anchor plus `data-part` regions.
  **Severity: medium.**
- A route template with a new root element that lacks a stable id while
  adding route-specific CSS. **Severity: medium.**
- Hardcoded spacing, surface, font, radius, or z-index values where an
  appropriate Noora variable exists. **Severity: low.**

### Do not flag

- Plain Noora component usage that does not need extra CSS.
- Pixel dimensions for assets or visual effects where Noora has no
  matching variable.
- The anchor class itself for reusable layouts or components, such as
  `.layout`, `.headerbar`, or `.account-dropdown`.

---

## 3. Auth and session boundaries

`HIVE_VISIBILITY=public|private` controls whether the dashboard is
gated. Public instances allow everyone through, while private instances
require a signed-in session. Login remains available in both modes.

### Flag

- A route that should be dashboard-protected but bypasses
  `HiveWeb.Plugs.RequireAuthenticated` or its existing router pipeline.
  **Severity: high.**
- A callback path that stores a user in the session before validating the
  provider's allowed-domain rules via `Hive.Auth.check_domain/2`.
  **Severity: high.**
- Provider logic that bypasses `Hive.Auth` and reads raw auth config
  directly from controllers or templates. **Severity: medium.**
- OIDC provider code that hardcodes discovered endpoints instead of
  relying on `ueberauth_oidcc` issuer discovery. **Severity: medium.**
- Google auth changes that remove the server-side domain check and rely
  only on the `hd` authorize hint. **Severity: high.**
- Dynamic OAuth registration or another public unauthenticated endpoint
  that trusts nested metadata to be the expected shape and can crash on
  malformed user input instead of returning a controlled error or
  normalizing to a safe default. **Severity: medium.**
- Dynamic OAuth registration changes that allow server-side URL fetching
  from user-controlled metadata such as `jwks_uri`, unless the diff also
  adds explicit SSRF-safe validation and tests. **Severity: high.**

### Do not flag

- Login being reachable when visibility is public. That is intentional
  so admins can sign in to a public instance.
- Compile-time Ueberauth provider declarations in `config/config.exs`.
  Runtime credentials are populated separately in `config/runtime.exs`.

---

## 4. Test setup boundaries

Tests run async-by-default and stub through Mimic, not `Application`
config. The static parts of this rule are enforced by credo; what's left
here is the cross-file and semantic piece.

### Flag

- A new mockable module used with Mimic (`stub/3`, `expect/3`) that is
  not added to `test/test_helper.exs` via `Mimic.copy/1`. The first
  stubbing test will silently no-op. **Severity: medium.**
- Tests for a new route or controller action that are not mirrored under
  the matching `test/hive_web/...` path. **Severity: low.**

### Do not flag

- Process-local Mimic stubs of modules already copied in
  `test/test_helper.exs`.
- Database setup done through the existing async-safe ConnCase/DataCase
  patterns.

---

## 5. Helm and production overlay boundaries

The Helm chart must remain generic by default. Tuist-specific production
values belong in `infra/helm/hive/values-production.yaml`.

### Flag

- Generic chart templates or `values.yaml` that hardcode
  `hive.tuist.dev`, `tuist.dev`-specific auth domains, the
  `hive-k8s-production` vault, Hetzner-specific storage classes, or
  production-only External Secrets settings. **Severity: high.**
- `values-production.yaml` changes that inline real secret values instead
  of 1Password remote references. **Severity: critical.**
- Deployment workflow changes that print kubeconfig contents, secret
  values, OAuth credentials, database URLs, or ExternalSecret payloads in
  logs. **Severity: critical.**
- Helm changes that drop the production smoke test, rollback behavior, or
  stuck-release recovery without replacing them with an equivalent safety
  mechanism. **Severity: medium.**

### Do not flag

- Tuist-specific values inside `infra/helm/hive/values-production.yaml`
  when they are references or non-secret deployment metadata.
- Generic chart support for External Secrets as long as values remain
  configurable and disabled unless the deployer opts in.

---

## 6. Releases and commit scopes

Hive has two release tracks: app releases from non-`helm` Conventional
Commit scopes and Helm chart releases from `helm`-scoped commits.

### Flag

- Release workflow changes that let Helm-only commits contribute to the
  app changelog, or non-Helm commits contribute to the Helm changelog.
  **Severity: medium.**
- Changes that remove the `helm@X.Y.Z` tag prefix from release
  detection or tagging. **Severity: high.** (App tags are plain
  `X.Y.Z`; the legacy `server@X.Y.Z` form is still accepted by the
  workflow's detection logic for backward compatibility.)
- PR titles or commit-message guidance that omits explicit Conventional
  Commit scopes when documenting examples for this repo. **Severity:
  low.**

### Do not flag

- Existing `git-cliff` behavior that accepts unscoped commits. The repo
  convention still prefers explicit scopes, but release parsing tolerates
  them.

---

## 7. LiveView lifecycle

The forage, dashboard, domain, and spec sections use Phoenix LiveView.
These rules catch lifecycle bugs that need semantic context (which
routes are tenant-scoped, what counts as "slow", which collections grow)
and so live outside credo.

### Flag

- A new LiveView route on a tenant-scoped resource whose `mount/3` does
  not gate access via the corresponding LetMe policy (e.g.
  `Hive.Forage.Policy`). **Severity: high.**
- A `mount/3` performing a fetch that can exceed ~100ms (DB query,
  agent run, external HTTP) without `Phoenix.LiveView.assign_async/3`.
  **Severity: medium.**
- A new LiveView that assigns an unbounded collection
  (e.g. `assign(socket, :specs, list)`) instead of
  `Phoenix.LiveView.stream/3`. **Severity: medium.**
- A `handle_event/3` performing a slow operation (DB write, agent run,
  external HTTP) inline instead of offloading to a Task or background
  process. **Severity: medium.**
- New domain logic (data access, multi-step flows, business rules)
  placed inside a controller, LiveView `mount/3`/`handle_event/3`, or
  HEEx template instead of a context module under `lib/hive/<domain>/`.
  The web layer should orchestrate; the domain should compute.
  **Severity: medium.**

### Do not flag

- Existing controller-rendered HEEx templates where the page does not
  need LiveView interactivity.
- LiveViews that legitimately need synchronous data at mount (page
  title, OpenGraph metadata) when the fetch is bounded and fast.
- Thin presentation helpers defined in the LiveView module itself
  (formatting, derived assigns) that don't reach for the DB or external
  systems.

---

## 8. Ecto changesets, tenancy, and templates

These rules require diff awareness or cross-file tracing that credo
cannot do.

### Flag

- A new or materially changed `*_changeset/N` function in `lib/hive/**`
  whose diff does not also add or update a test calling that function in
  the matching `test/hive/**/*_test.exs`. Material changes include any
  new/modified `cast`, `validate_*`, `unique_constraint`,
  `foreign_key_constraint`, or `put_change` line. **Severity: medium.**
- A changeset that `cast`s a programmatic foreign-key field (`user_id`,
  `account_id`, `organization_id`, `actor_id`, or any FK naming the
  actor/owner) from user params instead of setting it via `put_change`
  from a verified actor. **Severity: high.**
- A controller, LiveView, or context function calling
  `Hive.Repo.get(Schema, id)` / `Repo.one(from s in Schema, where: s.id == ^id)`
  for a tenant-owned schema without a tenant constraint, when the call
  site already has the tenant in scope. **Severity: high.**
- A HEEx template accessing `@record.assoc.field` with no upstream
  evidence (trace through the context function) that the association was
  preloaded. The render-time crash on `%Ecto.Association.NotLoaded{}` is
  silent at compile time. **Severity: medium.**
- New code that normalizes user-controlled params, JSON payloads, form
  attrs, webhook attrs, or external API payloads by converting arbitrary
  string keys to atoms, including `String.to_existing_atom/1`. Use an
  explicit key whitelist and ignore or reject unknown keys instead.
  **Severity: high.**
- New in-memory request-facing counters, rate limiters, nonce stores, or
  ETS tables that grow by client, token, IP, route, or time bucket
  without a bounded retention or pruning path. **Severity: medium.**
- New or changed XML/HTML feed/rendering helpers that duplicate escaping
  logic across formats instead of sharing one escaping helper, or that
  interpolate user-controlled text into XML/HTML without escaping.
  **Severity: high.**

### Do not flag

- Trivial mechanical changeset edits (renaming a field already covered
  by an existing test, formatting-only churn, reordering pipe steps).
- Diffs that exercise the changeset indirectly through a higher-level
  context test that still passes.
- Closed-world atom conversion for trusted internal identifiers, such as
  a route provider key that is immediately checked against configured
  providers.
- Long-lived ETS tables whose key space is fixed and small, or stores
  with an existing external TTL/cleanup owner.

---

## 9. Documentation and operator-facing surfaces

`README.md` describes behavior, configuration, and outcomes for the
operator or end user of any Hive deployment. Tuist-specific operational
details (production cluster, 1Password vault names, ESO config, Hetzner
storage classes, deployment workflow internals) belong in `AGENTS.md`,
not the README. Implementation details — module names, struct fields,
private helpers, refactoring artifacts — belong in code or `AGENTS.md`,
never in user-facing docs.

### Flag

- A PR that adds, changes, or removes a feature visible to operators or
  end users (new env var, new route, new ingestion source, changed
  default, removed capability) without updating `README.md` in the same
  diff. **Severity: medium.**
- A `README.md` change that introduces Tuist-specific operational
  details (vault names, cluster names, ESO config, hcloud storage
  classes, production-only workflow notes) instead of putting them in
  `AGENTS.md`. **Severity: low.**
- A `README.md` change that leaks implementation details (module names,
  function signatures, private struct fields, internal refactoring
  notes) instead of describing observable behavior. **Severity: low.**
- A doc cross-reference to something that no longer exists or now works
  differently after the diff (stale env var, removed route, renamed
  flag). **Severity: medium.**

### Do not flag

- Internal-only changes (refactors, perf, ops-only paths, tests,
  fixtures) that have no operator- or user-visible effect.
- Existing implementation references in `AGENTS.md` — that file is
  explicitly the place for them.
- README additions for genuinely user-facing operational concerns
  (allowed-domain configuration, visibility modes) even if Tuist also
  uses them.

---

## Before submitting findings

For each finding, confirm:

1. The `path:line` is real and the snippet appears in the diff.
2. The category above is one of 1–9; if it isn't, downgrade to a
   question (`uncertain: ...`) rather than asserting a finding.
3. The severity is set: **critical** (auth bypass / cross-tenant read or
   write / secret leak), **high** (likely security or correctness bug),
   **medium** (compliance / consistency gap), **low** (nice-to-have).
4. You are not reporting an unchanged line as a finding. Unchanged
   context can explain a diff finding, but cannot be the finding itself.

---

## Out of scope (handled elsewhere — do not flag)

- Module/function naming, pipe-chain start, nesting depth, parentheses
  on no-arg calls → `mix format` and built-in credo checks.
- Migration `timestamps(...)` or schema `timestamps(...)` without
  explicit `type:` → `Hive.Credo.Checks.TimestampsType`.
- `Repo.*` calls inside `Enum.*` / `Stream.*` / `for` (N+1 shape) →
  `Hive.Credo.Checks.RepoCallInEnum`.
- Missing `@spec` / `@type` — this codebase intentionally avoids
  typespecs. Never suggest adding them.
- Missing `@doc` / `@moduledoc` on internal helper modules.
- Obvious `String.to_atom/1` on user input → credo's `UnsafeToAtom`.
  Keep review-time findings for semantic cases such as
  `String.to_existing_atom/1` hidden inside external payload
  normalization, where a whitelist is expected.

---

## Skill maintenance (not a review-time instruction)

A rule belongs in this skill **only** when it needs at least one of:

- **The PR diff** ("changed in this PR", "co-changed with X").
- **Semantic context** that depends on the project ("this schema is
  tenant-owned", "this route is public", "this is a request path").
- **Cross-file or cross-module reasoning** ("was this association
  preloaded upstream", "is the mock registered in `test_helper.exs`").
- **Human-shaped pattern recognition** ("this string looks like a real
  credential", "this leaks implementation details").
- **External-system awareness** that the code itself doesn't expose
  (Helm overlays, 1Password references, deployment workflow).

A rule that reduces to "match this AST shape in files under this path"
belongs in `credo/checks/`, not here. When the temptation is to add a
new section to this skill, first ask: could a Credo check do it? If
yes, write the check instead and leave only the semantic escalation
(severity by context, hot-path judgment) for the skill.