.blick/skills/tuist-elixir-review/SKILL.md
This skill is intentionally narrow. Generic Elixir style, naming, pipe
chains, formatting, nesting depth, and String.to_atom/1-style hygiene
are already covered by mix format and credo in CI — do not flag
those. Focus on the rules below; they catch real bugs.
For each finding, cite path:line (or Module.function/arity) and
quote the relevant snippet.
lib/tuist/authorization.ex + AuthorizationPlugThe policy DSL is LetMe.Policy. Categories are declared via object :foo do ... end blocks. The API plug at
server/lib/tuist_web/plugs/api/authorization/authorization_plug.ex
hard-codes which categories are project-scoped:
@project_categories [:run, :bundle, :cache, :preview, :test, :build, :automation_alert]
object :foo do block (project-scoped resource) without :foo being added to @project_categories in authorization_plug.ex. The route guard will silently miss the new category. Severity: high.action that uses [:authenticated_as_project, ...] but omits :projects_match. This lets an authenticated project act on another project's resource. Severity: critical. The canonical correct form is allow([:authenticated_as_project, :projects_match]).:public_project or [:authenticated_as_user, :ops_access] allowed for :create, :update, or :delete actions. These flags are intended for read-only paths.action :read | :create | :update | :delete that doesn't cover all three subject kinds (:authenticated_as_user, :authenticated_as_project, :authenticated_as_account with a scopes_permit: check) without an inline desc(...) explaining the omission. Missing one is usually a bug; an explicit desc is the documented escape hatch.allow without a scopes_permit: check (e.g. bare [:authenticated_as_account]). Account tokens must always be scope-gated.object/action blocks unchanged by the diff.allow(...) lines within an action.Repo.get on multi-tenant schemasTenant-owned schemas include at least: Bundle, Run, Cache,
Preview, CommandEvent, Build, Test, Project,
AutomationAlert. They all carry a project_id or account_id.
Tuist.Repo.get(Schema, id) / Repo.one(from(s in Schema, where: s.id == ^id)) without a project_id / account_id constraint for any of the schemas above, when the call is inside a controller, plug, LiveView, channel, MCP handler, or worker that already has the project/account in scope. This is a tenant leak (an attacker who guesses a UUID gets cross-tenant data). Severity: high.id and forwards it to Repo.get without also taking the project/account.# admin / cross-tenant: ... comment or a function name like *_for_all/_global/_admin).User, Account, Organization, Subscription, etc.).IngestRepo is write-onlyThere are two ClickHouse repos in this codebase, and they are not interchangeable. Be precise about which one a call uses before flagging.
Tuist.IngestRepo — write-only ingest path. Application code must
not read from it; reads happen out of band.Tuist.ClickHouseRepo — the read-only ClickHouse repo (declared
with read_only: true in server/lib/tuist/clickhouse_repo.ex).Tuist.IngestRepo.all/1, Tuist.IngestRepo.get/2,
Tuist.IngestRepo.get_by/2, Tuist.IngestRepo.one/1,
Tuist.IngestRepo.exists?/1, or Tuist.IngestRepo.aggregate/3.from(... ) |> Tuist.IngestRepo.<read fn>.The fix is almost always to read through Tuist.ClickHouseRepo (for
ClickHouse-only data) or Tuist.Repo (PostgreSQL).
Tuist.ClickHouseRepo.all/1, Tuist.ClickHouseRepo.one/1,
Tuist.ClickHouseRepo.aggregate/3, or any other read through
ClickHouseRepo. That repo exists specifically for application reads.
Do not confuse it with IngestRepo.Tuist.IngestRepo.insert/2 / insert_all/2,3 — those are
the intended use.test_helper.exsserver/test/test_helper.exs is the single place where
Mimic.copy(Module) is called. Per-test-file Mimic.copy/1 calls leak
state across tests and are an explicit anti-pattern in this repo.
Mimic.copy(...) call inside any file under server/test/ other
than test_helper.exs. Suggest: move it to test_helper.exs.Mimic.expect/3, Mimic.stub/3, Mimic.reject/1 — those belong in tests.Mimic.copy/1 calls in test_helper.exs itself.In server/priv/repo/migrations/ and server/priv/ingest_repo/migrations/:
timestamps() without
type: :utc_datetime_usec and a corresponding migration column
without :timestamptz. The .credo.exs rule says: migrations use
:timestamptz, schemas (lib/) use :utc_datetime.add :inserted_at, :naive_datetime or :datetime without timezone in a migration. Should be :timestamptz.data-export.md updates on schema changesserver/data-export.md documents every piece of customer data Tuist
stores, for GDPR Article 20 / CCPA exports. It must be updated when
the diff includes any of:
server/lib/tuist/**/*.ex that maps to a
customer-facing tablebundles/,
previews/, caches/)server/priv/repo/migrations/*.exs (other than
pure index / constraint changes) or server/priv/ingest_repo/migrations/*.exs
without also modifying server/data-export.md.This is a compliance gap, not just a docs nit — call it out clearly.
In marketing copy and pricing UI:
€, $, £, ¥, currency codes) wrapped inside
dgettext/2 or gettext/1. Symbols and amounts must remain identical
across languages.dgettext("marketing", "0€ and up"). The
correct form keeps the currency literal outside the translation:
"0€ " <> dgettext("marketing", "and up")..po is read-only for humansserver/priv/gettext/**/*.po. Only the tuistit
bot may edit .po files; CI will fail otherwise.mix gettext.extract --merge. Only the no---merge form is
allowed in PRs..pot (template) changes — those are produced by mix gettext.extract
and are expected when adding new translatable strings.mix format + credo (PipeChainStart,
StrictModuleLayout, Nesting, UnsafeToAtom, ModuleDoc).@spec / @type — this codebase intentionally avoids
typespecs. Never suggest adding them.@doc / @moduledoc on internal helper modules.String.to_atom/1 on user input — credo's UnsafeToAtom covers it.For each finding, confirm:
path:line is real and the snippet appears in the diff.uncertain: ...) rather than asserting a finding.