bcquality/microsoft/skills/review/al-code-review.md

kind	id	version	title	description	inputs	outputs	bc-version	technologies	countries	application-area	sub-skills
action-skill	al-code-review	1	AL code review	Reviews AL source changes by composing the AL review leaf skills (performance, security, privacy, upgrade, style, UI).	pr-diff file-path	findings-report	all	al	w1	all	microsoft/skills/review/al-performance-review.md microsoft/skills/review/al-security-review.md microsoft/skills/review/al-privacy-review.md microsoft/skills/review/al-upgrade-review.md microsoft/skills/review/al-style-review.md microsoft/skills/review/al-ui-review.md

AL code review

Reviews AL source changes by composing the leaf AL review skills. This is the canonical reference implementation of a super-skill — skill authors writing composed reviews should copy its structure.

al-code-review does not evaluate knowledge files directly. It invokes each of its sub-skills against the same task input, collects their findings-reports, and returns a rolled-up findings-report.

An orchestrator invokes this skill with either a pr-diff (the standard PR-review entry point) or a file-path (single-file review). The skill produces a single JSON document conforming to the DO output contract, extended with sub-results and — when applicable — skipped-sub-skills.

Source

The sub-skills invoked by this skill are those listed in frontmatter sub-skills:

• microsoft/skills/review/al-performance-review.md
• microsoft/skills/review/al-security-review.md
• microsoft/skills/review/al-privacy-review.md
• microsoft/skills/review/al-upgrade-review.md
• microsoft/skills/review/al-style-review.md
• microsoft/skills/review/al-ui-review.md

Additional leaf skills (for example, telemetry, testing) are added by updating the sub-skills list. The skill does not discover sub-skills implicitly.

Relevance

A sub-skill is relevant when both of the following hold:

• The orchestrator has supplied inputs that satisfy the sub-skill's declared inputs.
• The orchestrator has not disabled the sub-skill via configuration.

Per the DO contract, the super-skill MUST NOT filter sub-skills by task content. al-code-review does not inspect the PR diff to predict whether, for example, there is anything for al-security-review to find. Each leaf is responsible for its own task-level applicability decision; leaves signal non-applicability by returning outcome: "not-applicable" or outcome: "no-knowledge".

Sub-skills that fail either check are not invoked and are recorded in skipped-sub-skills:

• reason: "configuration" when the orchestrator disabled the sub-skill.
• reason: "not-applicable" when the orchestrator's inputs do not satisfy the sub-skill's declared inputs.

Worklist

The worklist is the list of sub-skills judged relevant by the previous step. Every sub-skill in the worklist will be invoked in the Action step.

Action

For each sub-skill in the worklist:

1. Invoke the sub-skill with the orchestrator's inputs, passing only the subset each sub-skill declares in its inputs.
2. Capture the sub-skill's complete findings-report verbatim and append it to sub-results.
3. If the sub-skill's outcome is failed, stop here for this sub-skill: its findings are not reliable per the DO contract and MUST NOT be copied into the super-skill's top-level findings[] or counted in summary.counts.
4. Otherwise, append each entry from the sub-skill's findings[] to the super-skill's top-level findings[], setting from-sub-skill to the sub-skill's skill.id. For non-citation findings (those whose id is a skill-defined slug rather than a reference path), prefix id with <from-sub-skill>: to prevent collisions across sub-skills. Other finding fields are preserved.

Aggregate summary.counts and summary.coverage as the sums across invoked sub-skills whose outcome is not failed.

suppressed[] at the super-skill level remains empty. Knowledge-file-level suppression is reported by each sub-skill within its own entry in sub-results.

Derive outcome using the DO rollup rules. outcome-reason is populated for partial and failed and SHOULD summarize per-sub-skill state, for example: "al-security-review failed (tool timeout); al-performance-review completed."

Output

Output conforms to the DO output contract, extended with sub-results and skipped-sub-skills. A populated example — both leaves ran, each produced findings:

{
  "skill": { "id": "al-code-review", "version": 1 },
  "outcome": "completed",
  "summary": {
    "counts": { "blocker": 1, "major": 1, "minor": 2, "info": 0 },
    "coverage": { "worklist-size": 4, "items-evaluated": 4 }
  },
  "findings": [
    {
      "id": "microsoft/knowledge/performance/filter-before-find.md",
      "severity": "major",
      "message": "FindSet is called on a record variable without any prior SetRange/SetFilter. This forces a full-table scan.",
      "location": {
        "file": "src/Sales/PostingRoutines.Codeunit.al",
        "line": 140,
        "range": { "start-line": 140, "end-line": 144 }
      },
      "references": [
        { "path": "microsoft/knowledge/performance/filter-before-find.md" }
      ],
      "confidence": "high",
      "from-sub-skill": "al-performance-review"
    },
    {
      "id": "community/knowledge/performance/call-setloadfields-before-filters.md",
      "severity": "minor",
      "message": "SetLoadFields is called after SetRange. Per the referenced guidance the call must come before filters to be folded into the query plan.",
      "location": {
        "file": "src/Sales/PostingRoutines.Codeunit.al",
        "line": 152
      },
      "references": [
        { "path": "community/knowledge/performance/call-setloadfields-before-filters.md" }
      ],
      "confidence": "high",
      "from-sub-skill": "al-performance-review"
    },
    {
      "id": "microsoft/knowledge/security/use-secrettext-for-credentials.md",
      "severity": "blocker",
      "message": "A bearer token is declared as a Text parameter and passed through the HTTP request path as plain text. The referenced guidance requires credentials to flow as SecretText end-to-end.",
      "location": {
        "file": "src/Integration/ApiClient.Codeunit.al",
        "line": 85,
        "range": { "start-line": 85, "end-line": 89 }
      },
      "references": [
        { "path": "microsoft/knowledge/security/use-secrettext-for-credentials.md" }
      ],
      "confidence": "high",
      "from-sub-skill": "al-security-review"
    },
    {
      "id": "microsoft/knowledge/security/never-hardcode-secrets-in-al.md",
      "severity": "minor",
      "message": "An API key is assigned from a string literal rather than retrieved from IsolatedStorage or Key Vault at runtime.",
      "location": {
        "file": "src/Integration/ApiClient.Codeunit.al",
        "line": 201
      },
      "references": [
        { "path": "microsoft/knowledge/security/never-hardcode-secrets-in-al.md" }
      ],
      "confidence": "medium",
      "from-sub-skill": "al-security-review"
    }
  ],
  "suppressed": [],
  "sub-results": [
    {
      "skill": { "id": "al-performance-review", "version": 1 },
      "outcome": "completed",
      "summary": {
        "counts": { "blocker": 0, "major": 1, "minor": 1, "info": 0 },
        "coverage": { "worklist-size": 2, "items-evaluated": 2 }
      },
      "findings": [
        {
          "id": "microsoft/knowledge/performance/filter-before-find.md",
          "severity": "major",
          "message": "FindSet is called on a record variable without any prior SetRange/SetFilter. This forces a full-table scan.",
          "location": {
            "file": "src/Sales/PostingRoutines.Codeunit.al",
            "line": 140,
            "range": { "start-line": 140, "end-line": 144 }
          },
          "references": [
            { "path": "microsoft/knowledge/performance/filter-before-find.md" }
          ],
          "confidence": "high"
        },
        {
          "id": "community/knowledge/performance/call-setloadfields-before-filters.md",
          "severity": "minor",
          "message": "SetLoadFields is called after SetRange. Per the referenced guidance the call must come before filters to be folded into the query plan.",
          "location": {
            "file": "src/Sales/PostingRoutines.Codeunit.al",
            "line": 152
          },
          "references": [
            { "path": "community/knowledge/performance/call-setloadfields-before-filters.md" }
          ],
          "confidence": "high"
        }
      ],
      "suppressed": []
    },
    {
      "skill": { "id": "al-security-review", "version": 1 },
      "outcome": "completed",
      "summary": {
        "counts": { "blocker": 1, "major": 0, "minor": 1, "info": 0 },
        "coverage": { "worklist-size": 2, "items-evaluated": 2 }
      },
      "findings": [
        {
          "id": "microsoft/knowledge/security/use-secrettext-for-credentials.md",
          "severity": "blocker",
          "message": "A bearer token is declared as a Text parameter and passed through the HTTP request path as plain text. The referenced guidance requires credentials to flow as SecretText end-to-end.",
          "location": {
            "file": "src/Integration/ApiClient.Codeunit.al",
            "line": 85,
            "range": { "start-line": 85, "end-line": 89 }
          },
          "references": [
            { "path": "microsoft/knowledge/security/use-secrettext-for-credentials.md" }
          ],
          "confidence": "high"
        },
        {
          "id": "microsoft/knowledge/security/never-hardcode-secrets-in-al.md",
          "severity": "minor",
          "message": "An API key is assigned from a string literal rather than retrieved from IsolatedStorage or Key Vault at runtime.",
          "location": {
            "file": "src/Integration/ApiClient.Codeunit.al",
            "line": 201
          },
          "references": [
            { "path": "microsoft/knowledge/security/never-hardcode-secrets-in-al.md" }
          ],
          "confidence": "medium"
        }
      ],
      "suppressed": []
    }
  ]
}

The empty-corpus case — BCQuality's state until knowledge files land — rolls up to no-knowledge:

{
  "skill": { "id": "al-code-review", "version": 1 },
  "outcome": "no-knowledge",
  "summary": {
    "counts": { "blocker": 0, "major": 0, "minor": 0, "info": 0 },
    "coverage": { "worklist-size": 0, "items-evaluated": 0 }
  },
  "findings": [],
  "suppressed": [],
  "sub-results": [
    {
      "skill": { "id": "al-performance-review", "version": 1 },
      "outcome": "no-knowledge",
      "summary": { "counts": { "blocker": 0, "major": 0, "minor": 0, "info": 0 }, "coverage": { "worklist-size": 0, "items-evaluated": 0 } },
      "findings": [],
      "suppressed": []
    },
    {
      "skill": { "id": "al-security-review", "version": 1 },
      "outcome": "no-knowledge",
      "summary": { "counts": { "blocker": 0, "major": 0, "minor": 0, "info": 0 }, "coverage": { "worklist-size": 0, "items-evaluated": 0 } },
      "findings": [],
      "suppressed": []
    }
  ]
}