The full Salesforce review checklist

Supporting material for salesforce-code-review. Agents load it on demand; it ships inside the skill folder.

Raw Source

Verified 2026-09 against https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_gov_limits.htm.

Work the areas in order. Report only what you find.

1. Apex and governor limits

No SOQL, DML, callout, Database. method or Messaging.sendEmail inside a loop.
Queries are selective — a filter on an indexed field, and no full-table scan on a large object.
Collections are used for lookups: build a Map<Id, SObject> once, rather than querying per row.
No Trigger.new[0], and no method signature that forces the caller into a loop.
Heap: no unbounded collection accumulating across a batch; query only the fields needed.
CPU: no nested loop over two large collections where a map would do.
Callouts are not mixed with DML in the same transaction without the correct async pattern.
Async choice is right: Queueable for chaining, Batch for volume, @future only for legacy.
No hard-coded ids — queues, profiles and users are looked up; record type Ids come from Utils.getRecordTypeId(...) in a static final, not an inline describe call.
No hard-coded org-specific URLs or usernames.

2. Access and injection

Every class declares with sharing (or without sharing with a reason) — no class omits the keyword.
Queries returning data to users use WITH USER_MODE.
DML on user-supplied data uses AccessLevel.USER_MODE.
WITH SECURITY_ENFORCED is replaced, not supplemented, where found.
Dynamic SOQL uses Database.queryWithBinds with bind variables — no concatenation.
without sharing appears only with a comment justifying the elevation.
System-mode results returned to a user pass through Security.stripInaccessible.
@AuraEnabled and Apex REST methods validate every input and do not trust an id parameter.
Nothing guest-reachable grants more than intended.

3. Triggers and automation

One trigger per object, delegating to a handler — no logic in the trigger body.
Recursion guard where the trigger can cause its own re-entry.
The object's full automation inventory was reviewed: triggers, record-triggered Flows, remaining Process Builders, workflow rules, validation rules.
Before-save Flow and Apex trigger do not both write the same field.
No reliance on ordering between record-triggered Flows — it is not guaranteed.
Field updates that re-trigger automation are intentional and bounded.

4. Flows

No Get, Create, Update or Delete element inside a loop — collect into a collection variable and act once after the loop.
A fault connector on every element that can fail, going somewhere a human will see.
Entry criteria are tight enough that the Flow does not run on every save.
Before-save used for same-record field updates (no extra DML) rather than after-save.
Hard-coded ids absent; use custom metadata or a lookup.
Subflows do not hide a loop-with-DML one level down.

5. Lightning Web Components

Apex called imperatively has error handling and a user-visible failure state.
@wire errors are handled, not only the data branch.
No DOM access that Lightning Web Security restricts; no reaching outside the component.
Third-party libraries load from a static resource, never a CDN, with a Trusted URL configured.
loadScript/loadStyle are guarded so they run once.
SLDS 2 styling hooks are used rather than overriding internal SLDS classes, which break on upgrade.
Accessibility on primary flows: labels, roles, focus management, keyboard operability.
No sensitive data placed in component state that is visible in the DOM.

6. Tests

Assertions exist and are meaningful — not Assert.isTrue(true) or a bare System.assert.
A bulk case at 200 records, exercising the real entry point.
A negative case: invalid input, missing permission, validation failure.
A restricted-user case with System.runAs — an admin-only test proves nothing about access.
@TestSetup for shared data; no SeeAllData=true.
Test data is created by a factory, not copy-pasted per test.
Async work is asserted after Test.stopTest().
Callouts are mocked with HttpCalloutMock; the mock asserts the request, not only the response.

7. Permissions, metadata and packaging

Permission set diffs read line by line — especially "View All", "Modify All", "Author Apex", "Manage Users" and field-level security changes.
Profile changes justified; prefer permission sets for new grants.
No secret in custom metadata, custom settings or a static resource. Credential__c is acceptable when the field is encrypted and FLS is restricted to the integration user.
package.xml contains nothing unintended; destructive changes are deliberate and reviewed.
Named Credentials used for callout authentication, or Credential__c where the callout cannot use one.
New objects and fields have descriptions and sensible API names.

8. What the tooling should have caught

Run Code Analyzer v5 before reading, and treat its output as input to the review rather than the review itself:

sf code-analyzer run --workspace force-app --view detail

The Graph Engine finds CRUD/FLS violations across call paths; PMD finds the per-file patterns; RetireJS flags vulnerable JavaScript libraries in static resources; the Flow engine scans Flows. If a finding is a false positive, say so in the review with the reason rather than silently ignoring it.

# The full Salesforce review checklist

Verified 2026-09 against
<https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_gov_limits.htm>.

Work the areas in order. Report only what you find.

## 1. Apex and governor limits

- [ ] No SOQL, DML, callout, `Database.` method or `Messaging.sendEmail` inside a loop.
- [ ] Queries are selective — a filter on an indexed field, and no full-table scan on a large object.
- [ ] Collections are used for lookups: build a `Map<Id, SObject>` once, rather than querying per row.
- [ ] No `Trigger.new[0]`, and no method signature that forces the caller into a loop.
- [ ] Heap: no unbounded collection accumulating across a batch; query only the fields needed.
- [ ] CPU: no nested loop over two large collections where a map would do.
- [ ] Callouts are not mixed with DML in the same transaction without the correct async pattern.
- [ ] Async choice is right: Queueable for chaining, Batch for volume, `@future` only for legacy.
- [ ] No hard-coded ids — queues, profiles and users are looked up; record type Ids come from `Utils.getRecordTypeId(...)` in a `static final`, not an inline describe call.
- [ ] No hard-coded org-specific URLs or usernames.

## 2. Access and injection

- [ ] Every class declares `with sharing` (or `without sharing` with a reason) — no class omits the keyword.
- [ ] Queries returning data to users use `WITH USER_MODE`.
- [ ] DML on user-supplied data uses `AccessLevel.USER_MODE`.
- [ ] `WITH SECURITY_ENFORCED` is replaced, not supplemented, where found.
- [ ] Dynamic SOQL uses `Database.queryWithBinds` with bind variables — no concatenation.
- [ ] `without sharing` appears only with a comment justifying the elevation.
- [ ] System-mode results returned to a user pass through `Security.stripInaccessible`.
- [ ] `@AuraEnabled` and Apex REST methods validate every input and do not trust an id parameter.
- [ ] Nothing guest-reachable grants more than intended.

## 3. Triggers and automation

- [ ] One trigger per object, delegating to a handler — no logic in the trigger body.
- [ ] Recursion guard where the trigger can cause its own re-entry.
- [ ] The object's full automation inventory was reviewed: triggers, record-triggered Flows,
      remaining Process Builders, workflow rules, validation rules.
- [ ] Before-save Flow and Apex trigger do not both write the same field.
- [ ] No reliance on ordering between record-triggered Flows — it is not guaranteed.
- [ ] Field updates that re-trigger automation are intentional and bounded.

## 4. Flows

- [ ] No Get, Create, Update or Delete element inside a loop — collect into a collection variable and
      act once after the loop.
- [ ] A fault connector on every element that can fail, going somewhere a human will see.
- [ ] Entry criteria are tight enough that the Flow does not run on every save.
- [ ] Before-save used for same-record field updates (no extra DML) rather than after-save.
- [ ] Hard-coded ids absent; use custom metadata or a lookup.
- [ ] Subflows do not hide a loop-with-DML one level down.

## 5. Lightning Web Components

- [ ] Apex called imperatively has error handling and a user-visible failure state.
- [ ] `@wire` errors are handled, not only the data branch.
- [ ] No DOM access that Lightning Web Security restricts; no reaching outside the component.
- [ ] Third-party libraries load from a static resource, never a CDN, with a Trusted URL configured.
- [ ] `loadScript`/`loadStyle` are guarded so they run once.
- [ ] SLDS 2 styling hooks are used rather than overriding internal SLDS classes, which break on
      upgrade.
- [ ] Accessibility on primary flows: labels, roles, focus management, keyboard operability.
- [ ] No sensitive data placed in component state that is visible in the DOM.

## 6. Tests

- [ ] Assertions exist and are meaningful — not `Assert.isTrue(true)` or a bare `System.assert`.
- [ ] A bulk case at 200 records, exercising the real entry point.
- [ ] A negative case: invalid input, missing permission, validation failure.
- [ ] A restricted-user case with `System.runAs` — an admin-only test proves nothing about access.
- [ ] `@TestSetup` for shared data; no `SeeAllData=true`.
- [ ] Test data is created by a factory, not copy-pasted per test.
- [ ] Async work is asserted after `Test.stopTest()`.
- [ ] Callouts are mocked with `HttpCalloutMock`; the mock asserts the request, not only the response.

## 7. Permissions, metadata and packaging

- [ ] Permission set diffs read line by line — especially "View All", "Modify All", "Author Apex",
      "Manage Users" and field-level security changes.
- [ ] Profile changes justified; prefer permission sets for new grants.
- [ ] No secret in custom metadata, custom settings or a static resource. `Credential__c` is acceptable when the field is encrypted and FLS is restricted to the integration user.
- [ ] `package.xml` contains nothing unintended; destructive changes are deliberate and reviewed.
- [ ] Named Credentials used for callout authentication, or `Credential__c` where the callout cannot use one.
- [ ] New objects and fields have descriptions and sensible API names.

## 8. What the tooling should have caught

Run Code Analyzer v5 before reading, and treat its output as input to the review rather than the
review itself:

```bash
sf code-analyzer run --workspace force-app --view detail
```

The Graph Engine finds CRUD/FLS violations across call paths; PMD finds the per-file patterns;
RetireJS flags vulnerable JavaScript libraries in static resources; the Flow engine scans Flows.
If a finding is a false positive, say so in the review with the reason rather than silently ignoring
it.