references/manual-review-checklist.md
# Manual Review Checklist — governance & process
These are the Well-Architected criteria a local SFDX repo **cannot reveal**. The reviewer copies them into the report under **"MANUAL REVIEW — not auto-graded; assess with your team."** Do **not** score these from inference — they exist to hand the developer a structured governance checklist, not to fake a grade.
Copy the relevant blocks verbatim into the report as unchecked `[ ]` items.
---
## 🛡️ Trusted
### Secure (organizational / session)
- [ ] A **security matrix** maps every persona and integration to its authentication scheme and authorized data access.
- [ ] MFA enforced for interactive logins; user-to-entity is 1:1 (no shared accounts).
- [ ] Each integration uses a **unique API-only** integration user (not a person's account).
- [ ] Permission sets/groups are capability-based; profiles used minimally (IP ranges, login hours only).
- [ ] Session timeout ≤ 2h; clickjack, CSRF, XSS, content-sniffing, and redirect protections enabled.
- [ ] Credentials live in **Named Credentials**; nothing secret is readable in code or config.
- [ ] Encryption-at-rest (Shield / Hyperforce) applied where data sensitivity requires; in-transit protocols documented.
### Compliant
- [ ] Up-to-date **data dictionary** with Compliance Categorization, Data Owner, and Sensitivity Level on objects/fields.
- [ ] Data **residency** and cross-border replication strategy documented.
- [ ] Analytics granularity (region / nation / global) defined per compliance needs.
- [ ] **AI ethics:** training/RAG datasets are representative, complete, accurate, up-to-date.
- [ ] **AI ethics:** bias, explainability, and robustness assessed; "human at the helm" for high-risk / low-confidence cases.
- [ ] **AI transparency:** generative responses identify their data sources; bots are clearly identified to users.
- [ ] **AI monitoring:** data drift, fairness/bias scores, accuracy, and robustness tracked over time.
- [ ] **Accessibility:** tested across devices and assistive tech; keyboard focus visible; navigation not color-only; forms have labels, helpful errors, review/edit; translations in Translation Workbench.
### Reliable
- [ ] **Risk assessment** framework: risks categorized by people / process / technology and by customer impact; mitigation plans prioritized.
- [ ] Failure triggers and mitigation plans classified; controls automated early; mitigations revisited/improved.
- [ ] **Proactive Monitoring** / Scale Center enabled for anomaly detection; alerts integrated.
- [ ] **Scale / endurance testing** performed before business-critical events.
- [ ] **Archiving / purging** strategy documented and implemented for large data volumes; no data skew (>10k child/owner/lookup).
---
## ⚡ Easy
### Intentional
- [ ] Every work item has a clear, **measurable business-value** metric.
- [ ] Roadmaps prioritized by value, show dependencies and dates, and are maintained (not just kickoff slides).
- [ ] **Standard-vs-custom** decision principle documented: platform services → AppExchange → low-code → code.
- [ ] **Technical-debt registry** with deliverables, dates, and KPIs; trade-offs framed in business terms.
- [ ] Environment strategy and clear **hot-fix path** documented; approved AI models only.
- [ ] Documentation exists and is searchable: solution overview diagrams, **decision records** (options/trade-offs/rationale), code provenance logs.
### Automated
- [ ] Each automated process has **measurable, timebound** inputs/outputs and a named accountable stakeholder.
- [ ] Process KPIs surfaced in reports.
- [ ] "Fatal" vs "recoverable" error semantics defined; user-triggered automations notify users **before** committing changes.
### Engaging
- [ ] UX is streamlined — fewer clicks, clear value — informed by user research / journey mapping.
- [ ] Helpful, guided experiences for the primary personas.
---
## 🔁 Adaptable
### Resilient
- [ ] **Non-functional requirements / SLOs** defined for the next 1–3 years (the experience you want users to have, what you measure, over what period).
- [ ] Release management uses unlocked packages / DevOps Center / source-format deploys; **no failed-deployment** history.
- [ ] Clear release **cadence** with searchable, feature-tied release names; gen-AI quality thresholds defined per stage.
- [ ] Environment strategy: source-driven, source tracking on, metadata independent of release artifacts, **risk-based** release paths; no risky changes in prod at peak.
- [ ] **Recovery** protocols practiced regularly; service ownership and tooling known; recovery scripts repeatable; session-based permission sets for incident response.
- [ ] **Triage:** SMEs identified before incidents; handoff part of go-live; logging + custom error messages in execution paths.
- [ ] **Monitoring/alerting:** alerts only for human-intervention scenarios, delivered before failure, to those who can respond.
- [ ] **Business continuity plan** with triggers, steps, intervals, and dependencies; process + people areas tested; "recovery-first" mindset.
- [ ] **Backup:** strategy for both data **and** metadata; backups secured and human-readable; restores tested 2×/year in a Full/Partial Copy sandbox.
### Composable
- [ ] Naming conventions denote **functional units**; a defined list of units exists with change standards; landscape diagrams map components to units; units labeled in business terms.
- [ ] **State management:** stateful vs stateless use cases defined; Savepoints and rollback (Flow fault paths + Roll Back Records) used in data operations.
- [ ] **APIs** as predictable versioned contracts (one version per API), discoverable and documented; formats defined via CMT / platform events.
- [ ] **Messaging/eventing:** sync vs async use cases identified; data structures right-sized; platform events labeled; consistent patterns across Flow and code.
- [ ] **Dependency management:** hand-offs via messaging/eventing; dependency chains monitored; `LATEST` aliasing in `sfdx-project.json`; developers can build scratch orgs from source.
- [ ] **Packaging end goal:** loosely coupled packages; no unmanaged packages in prod/sandbox; org-dependent unlocked packages only for early-stage.
---
Source: Salesforce Well-Architected — https://architect.salesforce.com/docs/architect/well-architected/guide/overview.html
references/observable-checks.md
# Observable Checks — detection map
Each `[observable]` criterion from `well-architected-rubric.md`, mapped to **how to detect it** (preferred skill/MCP tool first, grep fallback) and **the anti-pattern it flags**. Delegate to the named skill when available; use the grep/Bash pattern for lightweight structural signals or when the skill/tool is unavailable.
Conventions below: `<pkg>` = a package directory from `sfdx-project.json` (usually `force-app`).
---
## 🛡️ Trusted
### Secure
| Criterion | Detection | Anti-pattern flagged |
|---|---|---|
| Sharing keyword on every class | `grep -rLE 'with(out)? sharing\|inherited sharing' --include='*.cls' <pkg>` (lists files with NO sharing keyword) | `ApexSharingViolations` — class omits sharing declaration |
| FLS/CRUD enforced | `dx-code-analyzer-run` (rule `ApexCRUDViolation`); grep `grep -rn 'WITH USER_MODE\|WITH SECURITY_ENFORCED\|stripInaccessible\|AccessLevel.USER_MODE' --include='*.cls' <pkg>` to confirm enforcement present | SOQL/DML with no FLS check |
| No SOQL injection | `dx-code-analyzer-run` (rule `ApexSOQLInjection`); grep dynamic SOQL: `grep -rn 'Database.query\|Database.getQueryLocator' --include='*.cls' <pkg>` then inspect for string concat vs `:bind` | User input concatenated into SOQL |
| No hard-coded secrets | `grep -rnE "(password\|secret\|api[_-]?key\|token)\s*=\s*'[^']+'" --include='*.cls' <pkg>` ; check for inline endpoint URLs | Credentials in code instead of Named Credentials |
| HTTPS callouts / safe crypto | `dx-code-analyzer-run` (`ApexInsecureEndpoint`, `ApexBadCrypto`); grep `grep -rn "'http://" --include='*.cls' <pkg>` | `http://` endpoints; hard-coded IV/key |
| Internal OWD not Public R/W | `platform-metadata-retrieve` + `sf org` inspection (org-connected) — inspect `<sharingModel>` in object metadata: `grep -rn '<sharingModel>' <pkg> ` | OWD ReadWrite without justification |
### Compliant *(mostly manual — weak file signals only)*
| Criterion | Detection | Anti-pattern flagged |
|---|---|---|
| Accessibility practice in LWC | grep LWC templates for inputs without `label`/`aria-`: `grep -rn '<input\|<lightning-input' <pkg> --include='*.html'` (inspect) | Inputs lacking labels; color-only cues |
| Translatable strings | grep for hard-coded user-facing text vs `$Label`: `grep -rn '{!\$Label\|@salesforce/label' <pkg>` (presence is a positive signal) | Hard-coded text instead of Custom Labels |
> Most Compliant criteria are governance — push to the manual checklist.
### Reliable
| Criterion | Detection | Anti-pattern flagged |
|---|---|---|
| Bulkified DML/SOQL | `dx-code-analyzer-run` (`OperationWithLimitsInLoop`) — primary | SOQL or DML inside a loop |
| Selective SOQL | `platform-lsp-integrate` → `check_soql_selectivity`; grep `grep -rniE 'LIKE .%|ALL ROWS' --include='*.cls' <pkg>` | Leading-wildcard `LIKE`, `ALL ROWS`, non-selective filters |
| No expensive ops in loops | `dx-code-analyzer-run` (`OperationWithHighCostInLoop`) | `Schema.describe*` / callouts in loops |
| Async for heavy work | grep `grep -rln 'implements Queueable\|implements Database.Batchable\|@future' --include='*.cls' <pkg>` (presence is positive; all-sync heavy logic is the smell) | Heavy/after-commit work forced synchronous |
---
## ⚡ Easy
### Intentional
| Criterion | Detection | Anti-pattern flagged |
|---|---|---|
| No legacy automation | `find <pkg> -name '*.workflow-meta.xml' -o -name '*.flowDefinition-meta.xml'` ; for Process Builder, grep flow files for `<processType>Workflow</processType>` | Workflow Rules / Process Builder still active |
| No `@future` (prefer Queueable) | `grep -rln '@future' --include='*.cls' <pkg>` | `@future` instead of Queueable |
| Modern API version | `grep -rn '<apiVersion>' <pkg> --include='*.xml' \| awk -F'[<>]' '$3 < 30'` ; check `sourceApiVersion` in `sfdx-project.json` | API version < 30.0 |
| No duplicated standard objects | `find <pkg> -path '*objects*' -name '*__c.object-meta.xml'` then compare base names to standard objects (Account, Contact, Case, Order, …) | `Account__c`, `Case__c` duplicating standard objects |
| Readable naming | Inspect class/object/field names for `Test1`, `tmp`, `asdf`, single letters | Throwaway/cryptic names in committed metadata |
| Documentation present | `grep -rL '/\*\*\|//' --include='*.cls' <pkg>` (files with no comments at all) | Undocumented classes |
### Automated
| Criterion | Detection | Anti-pattern flagged |
|---|---|---|
| Single-purpose Flows | Inspect Flow metadata count of distinct triggers/objects per flow | One flow doing many unrelated jobs |
| Try-catch around DML/SOQL/callouts | `grep -rLn 'try' --include='*.cls' <pkg>` cross-referenced with files doing DML/callouts | Unhandled DML/SOQL/callout |
| No debug in prod | `dx-code-analyzer-run` (`AvoidDebugStatements`); `grep -rn 'System.debug' --include='*.cls' <pkg>` | `System.debug` left in production paths |
| Sync-before / async-after | Inspect trigger handlers: DML in `after` contexts vs queued | DML in after-context blocking the transaction |
### Engaging *(manual — UX)*
| Criterion | Detection | Anti-pattern flagged |
|---|---|---|
| LDS over custom Apex for simple CRUD | grep LWC for `lightning/uiRecordApi` vs `@salesforce/apex`: `grep -rln 'lightning/uiRecordApi' <pkg>` | Hand-rolled Apex controllers for simple record CRUD |
---
## 🔁 Adaptable
### Resilient
| Criterion | Detection | Anti-pattern flagged |
|---|---|---|
| Source-tracked + CI | `test -d .git` ; `ls .github/workflows/*.yml 2>/dev/null` | No CI; not under source control |
| Source/package deploys (not change sets) | `ls manifest/package.xml` (past PoC = smell); confirm `packageDirectories` in `sfdx-project.json` | `package.xml`-driven deployment as the model |
| Meaningful tests | `find <pkg> -name '*Test.cls'` count vs class count; `grep -rln 'SeeAllData=true' --include='*.cls' <pkg>` ; check `System.assert` presence | No tests; `SeeAllData=true`; coverage-padding with no assertions |
### Composable
| Criterion | Detection | Anti-pattern flagged |
|---|---|---|
| Thin triggers → handlers | `grep -rEn 'SELECT\|insert \|update \|delete \|for *\(' --include='*.trigger' <pkg>` (logic/SOQL/DML/loops inside `.trigger` = smell) | Business logic in trigger body |
| CMT over Custom Settings | `find <pkg> -name '*.md-meta.xml'` (CMT, positive) vs custom settings: `grep -rln '<customSettingsType>' <pkg>` | Custom Settings / custom objects holding runtime config |
| Eventing for decoupling | `find <pkg> -name '*__e.object-meta.xml'` ; `grep -rln 'EventBus.publish' --include='*.cls' <pkg>` (presence positive where hand-offs exist) | Tight synchronous coupling for cross-domain hand-offs |
| Packageable + `LATEST` aliasing | `grep -n 'LATEST\|"versionNumber"' sfdx-project.json` ; check `packageDirectories` structure | Explicit pinned deps; no package strategy |
| No unmanaged packages committed | `grep -rln 'installedPackages' <pkg>` ; look for `*.installedPackage-meta.xml` | Unmanaged package artifacts in prod source |
| No duplicated metadata across pkg dirs | Compare component names across multiple `packageDirectories` paths | Same metadata in two package dirs |
---
## Mapping `dx-code-analyzer-run` severities → pillars
When `dx-code-analyzer-run` returns findings, map by rule:
| Rule | Pillar / Sub-pillar | Severity → review verdict |
|---|---|---|
| `ApexSOQLInjection` | Trusted / Secure | Critical → ❌ |
| `ApexCRUDViolation` | Trusted / Secure | High → ❌/⚠️ |
| `ApexInsecureEndpoint`, `ApexBadCrypto`, `ApexOpenRedirect` | Trusted / Secure | High → ⚠️ |
| `ApexSharingViolations` | Trusted / Secure + Adaptable / Composable | Moderate → ⚠️ |
| `OperationWithLimitsInLoop` | Trusted / Reliable + Easy / Automated | Moderate → ⚠️ (❌ if widespread) |
| `OperationWithHighCostInLoop` | Trusted / Reliable | Moderate → ⚠️ |
| `AvoidDebugStatements` | Easy / Automated | Low → note |
| `AvoidNonRestrictiveQueries` | Trusted / Reliable | Low → ⚠️ |
## Notes
- Prefer the skill/MCP tool over grep — `dx-code-analyzer-run` understands Apex structure; grep finds string patterns and over/under-reports. Use grep for structural/file-type signals (sharing keyword presence, legacy file types, deploy strategy) where a string match is reliable.
- Quote glob patterns passed to tools to stop the shell expanding them.
- A grep "hit count" is a signal, not a verdict — open the file and confirm before scoring a finding.
- Org-connected checks (OWD, permission sets) require `sf org display` to succeed; otherwise mark them manual.
references/well-architected-rubric.md
# Salesforce Well-Architected — Review Rubric
Distilled from the [Salesforce Well-Architected framework](https://architect.salesforce.com/docs/architect/well-architected/guide/overview.html). This is the criteria tree the `reviewing-architecture` skill scores against.
Each criterion is tagged:
- **`[observable]`** — gradable from the local repo (code + metadata) or a connected org. The reviewer scores these with `file:line` evidence. See `observable-checks.md` for the detection method.
- **`[manual]`** — a governance / process / human concern the repo cannot reveal. The reviewer lists these in the human checklist (`manual-review-checklist.md`); it never scores them from inference.
The framework has **three pillars**, each with **sub-pillars**:
> **🛡️ Trusted** protects stakeholders · **⚡ Easy** delivers value fast · **🔁 Adaptable** evolves with the business.
---
## 🛡️ TRUSTED
### Secure — organizational, session, and data security
- `[observable]` Every Apex class declares a sharing keyword (`with sharing` / `without sharing` justified / `inherited sharing`).
- `[observable]` SOQL/DML enforce FLS/CRUD — `WITH USER_MODE` / `WITH SECURITY_ENFORCED` on queries, `AccessLevel.USER_MODE` or `stripInaccessible` on DML.
- `[observable]` No SOQL injection — dynamic SOQL uses bind variables (`:var`), not string concatenation of user input.
- `[observable]` No hard-coded credentials/secrets in code; callouts use Named Credentials, not inline URLs/tokens.
- `[observable]` Callout endpoints use `https://` (no `ApexInsecureEndpoint`); crypto uses generated keys/IVs (no `ApexBadCrypto`).
- `[observable]` Internal OWD not Public Read/Write without justification *(org-connected)*.
- `[manual]` Security matrix maps every persona/integration to its authentication scheme and data access.
- `[manual]` MFA enforced; user-to-entity is 1:1 (no shared accounts); unique API-only user per integration.
- `[manual]` Session timeout ≤ 2h; clickjack/CSRF/XSS/content-sniffing protections enabled.
- `[manual]` Encryption-at-rest (Shield/Hyperforce) where data sensitivity requires it; protocols documented.
### Compliant — legal adherence, ethical standards, accessibility
- `[observable]` UI components show evidence of accessibility practice (labels on inputs, no color-only cues in LWC templates) — *partial signal only*.
- `[observable]` Translatable strings use Custom Labels / Translation Workbench, not hard-coded text — *partial signal*.
- `[manual]` Up-to-date data dictionary with Compliance Categorization, Data Owner, Sensitivity Level on objects/fields.
- `[manual]` Data residency / cross-border replication strategy documented.
- `[manual]` AI: datasets representative; bias/explainability/robustness assessed; "human at the helm" for high-risk cases.
- `[manual]` AI: generative responses identify their data sources; bots clearly identified to users; drift monitored.
- `[manual]` Accessibility tested across devices and assistive tech; keyboard focus visible; multilingual support where needed.
### Reliable — availability, performance, scalability
- `[observable]` Data operations bulkified — DML/SOQL operate on collections, not per-record in loops (`OperationWithLimitsInLoop`).
- `[observable]` SOQL is selective — indexed filters, no leading-wildcard `LIKE`, no `ALL ROWS`, minimal fields, no negative/`!=` filters that force table scans.
- `[observable]` No expensive operations in loops (`Schema.describe*`, callouts) — `OperationWithHighCostInLoop`.
- `[observable]` Async processing used for heavy/after-commit work (Queueable/Batch) rather than forcing it synchronous.
- `[observable]` No data-skew red flags in metadata (e.g. default-owner patterns) — *weak signal*.
- `[manual]` Risk assessment framework; failures categorized by people/process/technology and customer impact.
- `[manual]` Proactive Monitoring / Scale Center enabled; alerts integrated.
- `[manual]` Scale/endurance testing before business-critical events; archiving/purging strategy for LDV.
---
## ⚡ EASY
### Intentional — strategy, maintainability, readability
- `[observable]` No active legacy tech: Workflow Rules, Process Builder, `@future` (prefer Queueable), API version < 30.0, PushTopic/Generic events.
- `[observable]` No custom objects duplicating standard-object names/functionality.
- `[observable]` Consistent, human-readable naming across classes/objects/fields (no `Test1__c`, `tmp`, `asdf`).
- `[observable]` Code is documented — class/method headers, ApexDoc, non-trivial logic explained.
- `[manual]` Work items carry clear business-value metrics; roadmaps prioritized by value and maintained (not just kickoff slides).
- `[manual]` Standard-vs-custom decision principle documented (platform → AppExchange → low-code → code).
- `[manual]` Tech-debt registry with KPIs and dates; trade-offs framed in business terms.
- `[manual]` Solution overview diagrams + decision records (options, trade-offs, rationale) exist and are searchable.
### Automated — efficiency, data integrity
- `[observable]` Each Flow has a single purpose; main/subflow hierarchy; no hard-coded IDs in Flows.
- `[observable]` Each Apex class/method has a single, granular purpose (no god-classes).
- `[observable]` DML/SOQL/callouts wrapped in try-catch; custom exceptions used; Flows have fault connectors.
- `[observable]` No `System.debug` in production code paths (`AvoidDebugStatements`).
- `[observable]` Sync DML reserved for *before* contexts; async (Queueable/Batch) for *after*/heavy work.
- `[manual]` Process inputs/outputs measurable and timebound; accountable stakeholders named; metrics in reports.
### Engaging — streamlined, helpful experiences
- `[observable]` UI uses LWC with Lightning Data Service where appropriate (vs heavy custom Apex controllers for simple CRUD) — *partial signal*.
- `[manual]` UX streamlined — fewer clicks, clear value; user research / journey informs the design.
---
## 🔁 ADAPTABLE
### Resilient — ALM, incident response, continuity
- `[observable]` Source-tracked (git present); CI runs tests on commit (`.github/workflows/` or equivalent).
- `[observable]` Source/package-format deploys, not change sets; `package.xml` only for early-stage/PoC.
- `[observable]` Tests exist and are meaningful (TestDataFactory, not `SeeAllData=true`; assertions present, not just coverage-padding).
- `[manual]` Non-functional requirements / SLOs defined for the next 1–3 years.
- `[manual]` Release cadence with searchable, feature-tied release names; no history of failed deployments.
- `[manual]` Environment strategy (source-driven, source tracking, risk-based release paths); no risky changes in prod at peak.
- `[manual]` Recovery protocols practiced; BCP with triggers/steps/intervals incl. dependencies; backups tested 2×/year.
- `[manual]` Health-check APIs/SLOs; graceful degradation; alerts only for human-intervention scenarios, to those who can act.
### Composable — separation of concerns, interoperability, packageability
- `[observable]` Separation of concerns — thin triggers delegate to handler classes (no logic in triggers); UI ↔ logic ↔ data layered.
- `[observable]` Runtime config in Custom Metadata Types, not Custom Settings or custom objects.
- `[observable]` Platform Events / CDC used for decoupled hand-offs rather than tight synchronous coupling — *where eventing is present*.
- `[observable]` Packageable: source organized into package dirs; `LATEST` aliasing in `sfdx-project.json`; no unmanaged packages committed.
- `[observable]` No duplicated metadata across package directories.
- `[manual]` Naming conventions denote functional units; landscape diagrams map components to units.
- `[manual]` State management: stateful vs stateless patterns defined; Savepoints/rollback used in data operations.
- `[manual]` APIs as versioned contracts (one version per API), discoverable; messaging/eventing data structures right-sized.
- `[manual]` Dependency chains monitored; developers can build scratch orgs from source. End goal: loosely coupled packages.
---
## Scoring guidance
- A sub-pillar is **✅** when no anti-pattern is found across its `[observable]` criteria.
- **⚠️** when low/moderate findings exist, or only some criteria were observable (e.g. no org connected).
- **❌** when a critical/high anti-pattern is present (SOQL injection, FLS bypass, SOQL/DML-in-loop at scale, hard-coded secrets, logic-in-trigger).
- Roll sub-pillars up to a pillar verdict as **worst-of**, noting which sub-pillar drove it.
- Sub-pillars with **no** observable criteria gradable (e.g. Engaging, much of Compliant) are reported as **—** ("not auto-graded") and covered entirely by the manual checklist — this is expected, not a failure.
Source: Salesforce Well-Architected — https://architect.salesforce.com/docs/architect/well-architected/guide/overview.html
SKILL.md
---
name: platform-architecture-analyze
description: "Use when the developer asks to \"review the architecture\", \"run a Well-Architected check\", \"audit this project\", \"is this project well-architected?\", or wants to assess security/governor-limit/risk as a holistic code-and-metadata health report. Grades observable criteria (sharing/FLS, bulkification, SOQL selectivity, packageability) with file:line evidence; emits a governance checklist for what it can't see. Read-only, never edits. DO NOT TRIGGER for single-tool Apex scans (dx-code-analyzer-run)."
allowed-tools:
- Bash
- Read
- Glob
- Grep
metadata:
cliTools:
- tool: ["sf"]
semver: ">=2.0.0"
relatedSkills:
- "dx-code-analyzer-run"
- "platform-lsp-integrate"
- "platform-metadata-retrieve"
- "platform-apex-generate"
---
# Analyzing Architecture (Well-Architected Review)
Grade a Salesforce DX project against the **Salesforce Well-Architected** framework and produce an honest, evidence-backed report: a pillar-scored table for what's observable in code and metadata, plus a human checklist for the governance/process concerns a local repo can't reveal.
This skill is an **orchestrator**. It does not re-implement static analysis — it drives the analysis skills the plugin already ships and maps their output onto the Well-Architected pillars. It is **read-only**: it grades and recommends; it never edits, deploys, or deletes.
It also backs the `architecture-review` agent, which runs this exact workflow as a dedicated read-only reviewer. Invoke the agent for an end-to-end review; use this skill directly when you want the workflow inline in the current session.
## Capability resolution
1. **Skill-orchestrated review** (this skill) — runs the observable checks by delegating to existing skills/MCP tools, scores each sub-pillar, and emits the manual checklist.
2. **Direct CLI / grep** — used only for the lightweight structural signals the rubric names (sharing keywords, legacy-tech file types, deploy strategy). Fine standalone, but skips the pillar scoring and the governance checklist this skill provides.
3. **API** — not applicable.
## The rubric (read these first)
Before scoring, read the three reference files — they are the source of truth:
- [`references/well-architected-rubric.md`](references/well-architected-rubric.md) — the full pillar → sub-pillar → criteria tree, each criterion tagged `[observable]` or `[manual]`.
- [`references/observable-checks.md`](references/observable-checks.md) — each `[observable]` criterion mapped to its detection (skill / MCP tool / grep pattern) and the anti-pattern it flags.
- [`references/manual-review-checklist.md`](references/manual-review-checklist.md) — the `[manual]` criteria as a copy-pasteable governance checklist.
## Workflow
### Step 1 — Scope the project
```bash
# Package directories + API version
cat sfdx-project.json
```
Establish:
- **Package dirs** (from `packageDirectories[].path`) — where the source lives.
- **Inventory** — count Apex classes, triggers, LWC bundles, Aura, Flows, objects:
```bash
find <pkgdir> -name '*.cls' | wc -l
find <pkgdir> -name '*.trigger' | wc -l
find <pkgdir> -name '*.js-meta.xml' | wc -l # LWC bundles
```
- **Tooling signals** — does the repo have tests (`*Test.cls`, `__tests__/`), CI (`.github/workflows/`), linting (`.eslintrc*`, `.prettierrc*`), a `package.xml` vs source/package strategy?
- **Org connection** — `sf org display --json` succeeds → org-dependent checks (OWD, permission sets) are in play; otherwise mark them manual.
Record the scope line for the report header.
### Step 2 — Run the observable checks (delegate; don't re-scan)
Work through `references/observable-checks.md`. For the heavy lifting, delegate:
- **Apex security + performance** → `dx-code-analyzer-run`. It runs `sf code-analyzer` and classifies findings by severity. Map its rules onto the rubric:
- `ApexSOQLInjection`, `ApexCRUDViolation`, `ApexInsecureEndpoint`, `ApexBadCrypto` → **Secure**
- `ApexSharingViolations` → **Secure** (sharing) / **Composable** (separation)
- `OperationWithLimitsInLoop`, `OperationWithHighCostInLoop` → **Reliable** / **Automated**
- `AvoidDebugStatements` → **Automated**
- **Inline SOQL parse + selectivity, compile-level diagnostics** → `platform-lsp-integrate` (`apex_diagnostics`, `lwc_diagnostics`, `check_soql_selectivity`) when `lsp_health` is green → **Reliable** / **Automated**.
- **OWD / sharing model / permission sets** → `platform-metadata-retrieve` + `sf org` inspection, only if an org is connected → **Secure**.
For the lightweight structural signals, grep directly (patterns in `references/observable-checks.md`), e.g.:
```bash
# Secure — classes missing a sharing keyword
grep -rLE 'with(out)? sharing|inherited sharing' --include='*.cls' <pkgdir>
# Intentional — legacy tech still present
find <pkgdir> -name '*.workflow-meta.xml' -o -name '*.flowDefinition-meta.xml'
grep -rl '@future' --include='*.cls' <pkgdir>
# Composable — deploy strategy
ls manifest/package.xml 2>/dev/null # package.xml-driven (anti-pattern past PoC)
grep -l '"path"' sfdx-project.json # source/package strategy
# Composable — runtime config in custom settings vs CMT
find <pkgdir> -path '*objects*' -name '*.object-meta.xml' | xargs grep -l 'CustomSetting' 2>/dev/null
```
Collect every finding with `file:line` evidence. A check with no evidence is **not** a pass and **not** a fail — it's "not observable" and moves to the manual checklist.
### Step 3 — Score each observable sub-pillar
Assign ✅ / ⚠️ / ❌ per sub-pillar using the thresholds in `references/observable-checks.md`:
- **✅** no anti-patterns found in the observable checks for that sub-pillar.
- **⚠️** low/moderate findings, or only some criteria observable.
- **❌** critical/high findings (e.g. SOQL injection, FLS bypass, SOQL-in-loop at scale).
Then roll the sub-pillar verdicts up to a pillar verdict (worst-of, with a note).
### Step 4 — Emit the manual checklist
Copy the `[manual]` criteria from `references/manual-review-checklist.md` into the report as unchecked items, grouped by pillar. Label the section clearly: **"not auto-graded — assess with your team."** Do not guess at these; the point is to hand the developer a structured governance checklist, not to fake a score.
### Step 5 — Report
Produce one report using the format below. Lead with pillar verdicts, then observable findings (Trusted/Secure first — never bury security under style), then the manual checklist, then recommended next steps that name the skill which would apply each fix.
```text
Well-Architected Review — <project name>
Scope: <pkg dirs>, <N classes / M triggers / K LWC>, tests: <y/n>, CI: <y/n>, org: <connected alias / none>
PILLAR VERDICTS
🛡️ Trusted <✅|⚠️|❌> (Secure …, Compliant …, Reliable …)
⚡ Easy <✅|⚠️|❌> (Intentional …, Automated …, Engaging …)
🔁 Adaptable <✅|⚠️|❌> (Resilient …, Composable …)
OBSERVABLE FINDINGS (graded from code + metadata)
Sub-pillar | Verdict | Finding | Evidence (file:line / tool)
MANUAL REVIEW (not auto-graded — assess with your team)
[ ] <item> …
RECOMMENDED NEXT STEPS
- <highest-signal fix> → via `<skill>`
```
## Examples
### Example 1 — "Is this project well-architected?"
Scope the project, run all observable checks (delegating Apex analysis to `dx-code-analyzer-run`), score all three pillars, emit the full manual checklist, and report. This is the default full review.
### Example 2 — "Review my project's security and governor-limit risk"
Narrow to the **Secure** and **Reliable/Automated** sub-pillars: run `dx-code-analyzer-run` with a security + performance selector, use `platform-lsp-integrate` `check_soql_selectivity` for selectivity, grep for missing sharing keywords. Score those sub-pillars; still emit the Secure/Compliant manual items (security matrix, encryption strategy). Skip the Adaptable deep-dive unless asked.
### Example 3 — "Run a Well-Architected check before we package this for release"
Full review, weighting **Composable** (packageability — CMT vs custom settings, loose coupling, `LATEST` aliasing, no `package.xml`-driven deploys) and **Resilient** (source-tracked, CI, no failed deploys). Lead the report with the packageability readiness verdict.
## Failure modes
| Symptom | Cause | Recovery |
|---|---|---|
| `dx-code-analyzer-run` reports the analyzer isn't installed | Code Analyzer v5 missing | Note it in the report; fall back to grep-based structural checks for Apex and mark PMD-only criteria "not observable". |
| `sf org display` fails | No org connected | Mark OWD / permission-set / org-metadata criteria as manual; grade only file-based criteria. |
| LSP tools return `lsp_disabled` / `no_apex_workspace` | LSP off or no workspace | Skip the LSP-grounded checks; rely on `dx-code-analyzer-run` + grep. Note the gap. |
| No `sfdx-project.json` | Not an SFDX project | Stop — this skill reviews SFDX projects. Tell the developer. |
| Huge repo, scan is slow | Project-wide PMD + graph build | Scope `dx-code-analyzer-run` to the package dir; note that cross-file (sfge) findings may be partial. |
## Rules
- Read the three `references/*.md` files before scoring — the rubric is the source of truth.
- Delegate observable detection to existing skills/MCP tools; grep only for the lightweight structural signals the rubric names.
- Every observable finding carries `file:line` (or tool-result) evidence. No evidence → manual checklist, not the scored table.
- Never score a `[manual]` governance criterion from inference — list it for human review.
- Read-only: recommend fixes and name the skill that applies them (`platform-apex-generate` for Apex authoring / trigger refactoring); never edit, deploy, or delete.
- Lead with Trusted/Secure findings; don't bury security under style nits.
- Surface zero-finding sub-pillars briefly ("no issues found in observable checks") rather than omitting them.