Microsoft/bcquality
1
0
Fork 0
mirror of https://github.com/microsoft/BCQuality.git synced 2026-10-05 06:36:55 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Merge branch 'main' of https://github.com/demiliani/BCQuality into appsource

Browse source
This commit is contained in:
demiliani 2026-09-07 14:28:12 +02:00
commit 2ec109e534
204 changed files with 3165 additions and 404 deletions
Download patch file Download diff file Expand all files Collapse all files

6
.claude-plugin/marketplace.json
View file

@ -8,10 +8,10 @@
{
"name": "bcquality",
"source": "./",
"description": "Business Central AL quality knowledge base and review skills, packaged as an installable plugin. Ships the entire BCQuality tree (skills, knowledge, tools) so the Entry routing protocol runs against the installed clone.",
"version": "0.1.0",
"description": "Business Central AL quality knowledge base and review skills, packaged as an installable plugin. Exposes an AL review adapter while preserving BCQuality's internal Entry and action-skill protocols.",
"version": "0.2.0",
"skills": [
"./skills/bcquality-al-review/"
"./skills/"
]
}
]

33
.github/scripts/validate_frontmatter.py vendored
View file

@ -42,6 +42,7 @@ ACTION_SKILL_OPTIONAL_KEYS = {
}
META_SKILL_REQUIRED_KEYS = {"kind", "id", "version", "title"}
ENTRY_SKILL_REQUIRED_KEYS = {"kind", "id", "version", "title"}
HOST_SKILL_REQUIRED_KEYS = {"name", "description"}
STANDARD_INPUTS = {
"pr-diff", "object-list", "file-path", "repository", "telemetry-query",
@ -444,10 +445,36 @@ def validate_entry_skill(path: Path, parsed: Parsed, report: Report) -> None:
report.error(path, "R23", f"version must be a positive integer: {v!r}", 1)
def validate_host_skill(path: Path, parsed: Parsed, report: Report) -> None:
if parsed.frontmatter_error:
report.error(path, "R01", parsed.frontmatter_error, 1)
return
fm = parsed.frontmatter
assert fm is not None
missing = HOST_SKILL_REQUIRED_KEYS - fm.keys()
if missing:
report.error(path, "R29", f"missing required host-skill keys: {sorted(missing)}", 1)
name = fm.get("name")
if not isinstance(name, str) or not name:
report.error(path, "R29", "host-skill name must be a non-empty string", 1)
else:
if len(name) > 64 or not KEBAB_CASE.fullmatch(name):
report.error(path, "R29", f"host-skill name must be lowercase kebab-case and at most 64 characters: '{name}'", 1)
if name != path.parent.name:
report.error(path, "R29", f"host-skill name must match parent directory '{path.parent.name}', got '{name}'", 1)
description = fm.get("description")
if not isinstance(description, str) or not description:
report.error(path, "R29", "host-skill description must be a non-empty string", 1)
elif len(description) > 1024:
report.error(path, "R29", "host-skill description must be at most 1024 characters", 1)
# --- Path and sample checks -------------------------------------------------
def classify(path_from_root: Path) -> str | None:
"""Return 'knowledge' | 'action-skill' | 'meta' | 'entry' | None."""
"""Return 'knowledge' | 'action-skill' | 'host-skill' | 'meta' | 'entry' | None."""
parts = path_from_root.parts
if len(parts) < 2:
return None
@ -459,6 +486,8 @@ def classify(path_from_root: Path) -> str | None:
return "entry"
if name in META_SKILL_FILES:
return "meta"
if len(parts) == 3 and parts[2] == "SKILL.md":
return "host-skill"
return None
if top in LAYERS and path_from_root.suffix == ".md":
if len(parts) >= 3 and parts[1] == "skills":
@ -616,6 +645,8 @@ def run(root: Path) -> Report:
validate_entry_skill(path, parsed, report)
if parsed.frontmatter and isinstance(parsed.frontmatter.get("id"), str):
skill_records.append(SkillRecord(path, "entry-point", parsed.frontmatter["id"]))
elif kind == "host-skill":
validate_host_skill(path, parsed, report)
# Second pass: sample files per knowledge domain
for layer in LAYERS:

56
README.md
View file

@ -26,7 +26,7 @@ BCQuality contains **knowledge** and **skills**. It does not contain agents. Age
### Knowledge files
Atomic markdown files with YAML frontmatter. Each file covers one concern — one thing an agent would cite when reviewing or generating code. Knowledge files live in two layers:
Atomic markdown files with YAML frontmatter. Each file covers one concern — one thing an agent would cite when reviewing or generating code. Knowledge files live in three layers:
- **`/microsoft/`** — Microsoft-endorsed layer.
- `/microsoft/knowledge/` — Platform guardrails, official guidance.
@ -39,7 +39,9 @@ Atomic markdown files with YAML frontmatter. Each file covers one concern — on
- `/custom/knowledge/` — Organization-specific knowledge files.
- `/custom/skills/` — Organization-specific action skills.
All three layers are enabled by default when an agent consumes BCQuality. Content can be promoted from Community to Microsoft-endorsed once it proves itself — this is a first-class concept, not an afterthought.
All three layers are enabled by default when an agent consumes BCQuality. In the shared upstream layers, an action skill and the canonical knowledge it owns should live together: knowledge used by a Microsoft-endorsed skill belongs in `/microsoft/`, while `/community/` holds community-owned skills and their related knowledge. A split is acceptable briefly while a skill or corpus is being promoted, but it should not be the steady state. The `/custom/` layer remains the intentional exception because it overrides shared content in consumer forks.
Layer authority follows review and ownership, not the contributor's affiliation. Community contributions to a Microsoft-owned knowledge domain can therefore be accepted directly into `/microsoft/`; content can also be promoted from Community to Microsoft-endorsed once its owning skill is promoted.
### Skills
@ -60,6 +62,52 @@ Skills define how agents consume knowledge. They come in three flavors:
An orchestrator (such as AL-Go) points the agent at BCQuality's URL and provides a task context. The agent's first call is `/skills/entry.md`, which returns a dispatch record naming the action skill(s) to invoke. The agent then invokes each dispatched skill in turn, reading READ and DO on demand. No prior knowledge of BCQuality's structure is baked into the orchestrator — only the convention *"invoke `/skills/entry.md` first."*
### Standalone plugin installation
BCQuality can also be installed directly as a plugin. The plugin registers one
host-native skill,
[`al-code-review`](skills/al-code-review/SKILL.md), which adapts the caller's
request to the same Entry protocol used by orchestrators.
For GitHub Copilot CLI:
```shell
copilot plugin install microsoft/BCQuality
```
Plugin version `0.2.0` renamed the former `bcquality-al-review` skill to
`al-code-review`; explicit invocations and allowlists using the old skill name
must be updated. The name remains distinct from BC-ALAgents' public
`al-review` skill because current hosts may load plugin skill names into one
shared inventory.
The adapter is intentionally not a second review implementation:
```text
standalone host skill: skills/al-code-review/SKILL.md
-> routing contract: skills/entry.md
-> review coordinator: microsoft/skills/review/al-code-review.md
-> domain review leaves
```
Only the first file follows the host's `SKILL.md` packaging format. The
remaining files are BCQuality's internal protocol and layered action skills.
Entry remains the single owner of routing and index preparation;
`al-code-review.md` remains the single owner of broad-review composition. This
separation keeps standalone installation available without duplicating those
policies in the plugin adapter.
Note that a plugin install ships the entire tree, so `BCQUALITY_ENABLED_LAYERS`
narrows discovery without removing any files. Layer selection is a filter here,
not a deny mechanism — see [the adapter](skills/al-code-review/SKILL.md) for the
difference from the pruned-clone model.
The host adapter and internal action skill intentionally share the
`al-code-review` name: they expose the same operation in two different skill
formats. Their paths make the boundary explicit. The adapter lives under
`skills/al-code-review/SKILL.md`; the internal Microsoft-layer coordinator
lives at `microsoft/skills/review/al-code-review.md`.
## Knowledge file format
Every knowledge file is a markdown file with mandatory YAML frontmatter. Files target under 100 lines (ideal under 50). If two ideas would share a file, split them.
@ -90,7 +138,7 @@ Code examples belong in separate files, not in the knowledge file itself. Knowle
## Scope
The current curated corpus is focused on **technical AL code review**: AppSource and compatibility, data modeling, error handling, events, interfaces, performance, privacy, Query objects, security, style, telemetry, testing, UI, upgrade, and web services. These are the domains backed by knowledge files and registered review leaves today.
The current curated corpus is focused on **technical AL code review**: Agents, AppSource and compatibility, data modeling, error handling, events, interfaces, performance, privacy, Query objects, security, style, telemetry, testing, UI, upgrade, and web services. These are the domains backed by knowledge files and registered review leaves today.
Business Central functional domains (Finance, Supply Chain Management, Manufacturing, Jobs, Warehousing, Service), PowerShell, pipelines, and Power Platform remain valid future repository scope, but they are **not current coverage claims** until corresponding knowledge and action skills exist. Consumers should derive supported review scope from the live knowledge index and dispatched skills, not from roadmap breadth.
@ -148,7 +196,7 @@ Contributions are welcome. Before submitting a PR:
1. Read the knowledge file format above — frontmatter and sections are validated by CI.
2. Keep files atomic: one concern per file, under 100 lines.
3. Target your contribution to the right layer — most community contributions go in `/community/knowledge/`.
3. Target your contribution to the layer that owns the action skill: use `/microsoft/knowledge/` for Microsoft-owned domains and `/community/knowledge/` for knowledge that accompanies a community-owned skill.
4. Adding a BC fact — or stopping the agent from flagging a false positive — is a knowledge file, not a skill edit. If a PR changes *what* a review skill flags, the change almost certainly belongs in a knowledge file. See [`skills/write.md`](skills/write.md).
CI runs validation on every PR. If your knowledge file has schema violations, missing sections, code blocks, or exceeds 100 lines, the check will fail with a clear error message.

18
agent-consumption.md
View file

@ -12,6 +12,10 @@ For the high-level framing and repo structure, start with the [README](README.md
- **Global skills** in `/skills/` — the `entry.md` entry-point skill plus the READ · DO · WRITE contracts that govern the rest of the repo.
- **Layer content** in `/microsoft/`, `/community/`, and `/custom/` — knowledge files and action skills grouped by authority.
When BCQuality is installed as a standalone plugin, it additionally exposes
`skills/al-code-review/SKILL.md`. This is a host-format adapter, not another
action skill: it creates the task context and enters the same flow at Entry.
## The flow
```mermaid
@ -31,6 +35,12 @@ The orchestrator has a URL setting that points at BCQuality (default: `github.co
### 2. Agent invokes Entry
The agent reads `/skills/entry.md` and runs it against the task context. Entry applies its Source → Relevance → Worklist → Action steps over the action skills under `*/skills/**/*.md` and returns a **dispatch record**: the set of action skills to invoke, plus a list of candidates it skipped (with reasons). Routing is a skill, not orchestrator logic.
For a standalone plugin installation, the host activates the
`skills/al-code-review/SKILL.md` adapter first. That adapter preserves the
caller's actual goal, constructs the task context, and invokes Entry. It does
not select the internal `microsoft/skills/review/al-code-review.md` action skill
itself or duplicate Entry's preparation, routing, and failure semantics.
### 3. Agent consumes the dispatch record
The dispatch record names one or more action skills and the subset of inputs each should receive. If the outcome is `no-match` or `failed`, the agent returns the record to the orchestrator unchanged.
@ -48,7 +58,7 @@ Each action skill is a markdown file that specifies what to do at each step. The
| **Worklist** | Narrow from N candidates to the M that apply to this specific task. |
| **Action** | Apply the relevant knowledge and produce structured output. |
Example: a performance review skill sources from `/microsoft/knowledge/performance/` and `/community/knowledge/performance/`, filters to `bc-version: 26` and `technologies: [al]`, narrows the 25 candidate files to the 8 that apply to the 15 objects changed in the PR, and then evaluates each file against the diff.
Example: the Microsoft-owned performance review skill selects `performance` entries across every enabled layer, filters to `bc-version: 26` and `technologies: [al]`, narrows the candidate files to those that apply to the changed objects, and then evaluates each file against the diff. Its canonical corpus lives beside it under `/microsoft/knowledge/performance/`; cross-layer entries are limited to custom overrides or short-lived promotion work.
At this point the agent reads READ and DO on demand — it needs READ to interpret each knowledge file's frontmatter and sections, and DO to shape its output. Those contracts are fetched when first needed, not as part of bootstrap.
@ -89,8 +99,12 @@ Orchestrators MUST tolerate an absent `domain` in reports from older producers.
## Why this architecture
- **Entry is the only hardcoded thing.** Orchestrators ship with one convention — *"invoke `/skills/entry.md` first"* — and nothing else. New action skills and new knowledge files are picked up automatically because Entry discovers them at dispatch time.
- **Standalone installation adds an adapter, not another policy layer.** The
plugin's host-format `al-code-review` skill only translates the invocation
into Entry's task context. Entry and the dispatched action skills remain
authoritative.
- **Layers decide authority, not code.** The agent sees `/microsoft/` and `/community/` together; if two files conflict, the precedence rule defined in READ resolves it. A partner fork can disable `/community/` — that's a config choice, not a code change.
- **Knowledge and skills evolve independently.** A new knowledge file requires no skill changes — existing skills pick it up via frontmatter filters. A new skill requires no knowledge changes — it sources from what's already there.
- **Knowledge and skills evolve independently within their owning layer.** A new knowledge file requires no skill changes because existing skills pick it up via frontmatter filters. Layer placement still follows skill ownership, so promoting a skill also promotes its canonical corpus.
## The mental model, in one sentence

11
community/knowledge/agents/agent-permissions-intersect-with-assigner.bad.al Normal file
View file

@ -0,0 +1,11 @@
permissionset 50100 "SALES REVIEW AGENT"
{
Assignable = true;
Permissions =
tabledata "Sales Header" = RIM,
tabledata Customer = R,
tabledata User = RIMD,
tabledata "Access Control" = RIMD,
page "Sales Order" = X,
page "User Card" = X;
}

8
community/knowledge/agents/agent-permissions-intersect-with-assigner.good.al Normal file
View file

@ -0,0 +1,8 @@
permissionset 50100 "SALES REVIEW AGENT"
{
Assignable = true;
Permissions =
tabledata "Sales Header" = RIM,
tabledata Customer = R,
page "Sales Order" = X;
}

30
community/knowledge/agents/agent-permissions-intersect-with-assigner.md Normal file
View file

@ -0,0 +1,30 @@
---
bc-version: [27..]
domain: agents
keywords: [permissions, assigner, intersection, user-card, least-privilege]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Agent permissions intersect the assigner's; agents cannot configure users
## Description
An agent is a user, but it cannot configure users or other agents, and it cannot open sensitive pages such as user cards or permission-set assignment. Effective rights are the intersection of the assigning user's permissions and the agent's permission sets. Granting the agent a wide set does not bypass the assigner's limits, and a wide assigner still cannot give the agent user-admin powers the platform forbids.
## Best Practice
Document that intersection. Give the agent only the table and page rights its tasks need. Do not add user-setup or permission-assignment pages to the agent profile or permission sets; those operations will fail by design.
See sample: `agent-permissions-intersect-with-assigner.good.al`.
## Anti Pattern
Permission sets or profiles that include User card, Permission Set Assignment, or agent-admin pages, or comments that the agent runs as SUPER regardless of who assigned it. Detection signal: default access controls or profile including user-administration objects.
See sample: `agent-permissions-intersect-with-assigner.bad.al`.
## See also
`get-default-access-controls-least-privilege.md` covers the permission sets assigned when an agent instance is created.

8
community/knowledge/agents/agent-profile-narrows-visible-ui.bad.al Normal file
View file

@ -0,0 +1,8 @@
codeunit 50100 "Sales Review Agent Factory"
{
procedure GetDefaultProfile(var TempAllProfile: Record "All Profile" temporary)
begin
TempAllProfile."Profile ID" := 'BUSINESS MANAGER';
TempAllProfile.Insert();
end;
}

26
community/knowledge/agents/agent-profile-narrows-visible-ui.good.al Normal file
View file

@ -0,0 +1,26 @@
profile "SALES REVIEW AGENT"
{
Caption = 'Sales Review Agent';
Description = 'Restricted UI for the Sales Review Agent.';
RoleCenter = "Order Processor Role Center";
Customizations = "Sales Review Agent Sales Ord.";
}
pagecustomization "Sales Review Agent Sales Ord." customizes "Sales Order"
{
layout
{
modify("Payment Terms Code")
{
Visible = false;
}
}
actions
{
modify(Post)
{
Visible = false;
}
}
}

30
community/knowledge/agents/agent-profile-narrows-visible-ui.md Normal file
View file

@ -0,0 +1,30 @@
---
bc-version: [27..]
domain: agents
keywords: [profile, page-customization, hidden-actions, tooltip, role-center]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Give the agent a dedicated profile that hides unrelated UI
## Description
The agent only sees what its profile shows. Extra actions, views, and Role Center tiles become extra tools and extra tokens. Accuracy and cost both get worse as the UI widens. A human Order Processor profile is usually far too broad. Tooltips on the remaining actions are part of the tool description.
## Best Practice
Ship an agent-specific profile and page customizations: hide unrelated actions, keep descriptive tooltips, add Role Center links to the few pages the agent should open. Prefer fewer navigation hops.
See sample: `agent-profile-narrows-visible-ui.good.al`.
## Anti Pattern
Assigning `BUSINESS MANAGER` or `ORDER PROCESSOR` as `GetDefaultProfile` so the agent can do anything. Detection signal: default profile equal to a full-user role with no agent page customizations.
See sample: `agent-profile-narrows-visible-ui.bad.al`.
## See also
`get-default-profile-lives-in-the-app.md` covers packaging and assigning the profile that this rule narrows.

19
community/knowledge/agents/agent-setup-page-is-configuration-dialog.bad.al Normal file
View file

@ -0,0 +1,19 @@
page 50100 "Sales Review Agent Setup"
{
PageType = Card;
Caption = 'Set up Sales Review Agent';
SourceTable = "Sales Review Agent Setup";
layout
{
area(Content)
{
field(ReviewThreshold; Rec."Review Threshold")
{
ApplicationArea = All;
Caption = 'Review Threshold';
ToolTip = 'Specifies the threshold used when the agent requests a review.';
}
}
}
}

30
community/knowledge/agents/agent-setup-page-is-configuration-dialog.good.al Normal file
View file

@ -0,0 +1,30 @@
page 50100 "Sales Review Agent Setup"
{
PageType = ConfigurationDialog;
Caption = 'Set up Sales Review Agent';
SourceTable = "Sales Review Agent Setup";
SourceTableTemporary = true;
Extensible = false;
layout
{
area(Content)
{
part(AgentSetupPart; "Agent Setup Part")
{
ApplicationArea = All;
UpdatePropagation = Both;
}
group(AdditionalConfiguration)
{
Caption = 'Additional Configuration';
field(ReviewThreshold; Rec."Review Threshold")
{
ApplicationArea = All;
Caption = 'Review Threshold';
ToolTip = 'Specifies the threshold used when the agent requests a review.';
}
}
}
}
}

26
community/knowledge/agents/agent-setup-page-is-configuration-dialog.md Normal file
View file

@ -0,0 +1,26 @@
---
bc-version: [27..]
domain: agents
keywords: [configurationdialog, agent-setup-part, setup-page, pagetype, system-actions]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Agent setup pages use ConfigurationDialog and the Agent Setup Part
## Description
Instance setup is not a Card or StandardDialog. The toolkit expects `PageType = ConfigurationDialog` so OK and Cancel are system actions, plus the built-in `Agent Setup Part` for name, display name, state, and access. A Card with custom fields only drops those shared controls and the AI-use notices the part carries.
## Best Practice
Declare `PageType = ConfigurationDialog`, host `part(...; "Agent Setup Part")`, and put agent-specific fields in another group. Keep system OK/Cancel. Use a temporary source record and defer persistence until Update, as described in `agent-setup-source-table-is-temporary.md`. Following Microsoft's agent setup samples, set `Extensible = false`.
See sample: `agent-setup-page-is-configuration-dialog.good.al`.
## Anti Pattern
A Card or StandardDialog setup page with no `Agent Setup Part`. Detection signal: setup page ID from `IAgentFactory` / `IAgentMetadata` whose page is not `ConfigurationDialog` or has no `Agent Setup Part`.
See sample: `agent-setup-page-is-configuration-dialog.bad.al`.

29
community/knowledge/agents/agent-setup-source-table-is-temporary.bad.al Normal file
View file

@ -0,0 +1,29 @@
page 50100 "Sales Review Agent Setup"
{
PageType = ConfigurationDialog;
SourceTable = "Sales Review Agent Setup";
layout
{
area(Content)
{
field(ReviewThreshold; Rec."Review Threshold")
{
ApplicationArea = All;
Caption = 'Review Threshold';
ToolTip = 'Specifies the threshold used when the agent requests a review.';
trigger OnValidate()
begin
Rec.Modify(true);
end;
}
}
}
trigger OnOpenPage()
begin
if Rec.IsEmpty() then
Rec.Insert(true);
end;
}

68
community/knowledge/agents/agent-setup-source-table-is-temporary.good.al Normal file
View file

@ -0,0 +1,68 @@
page 50100 "Sales Review Agent Setup"
{
PageType = ConfigurationDialog;
SourceTable = "Sales Review Agent Setup";
SourceTableTemporary = true;
Extensible = false;
layout
{
area(Content)
{
part(AgentSetupPart; "Agent Setup Part")
{
ApplicationArea = All;
UpdatePropagation = Both;
}
group(AdditionalConfiguration)
{
Caption = 'Additional Configuration';
field(ReviewThreshold; Rec."Review Threshold")
{
ApplicationArea = All;
Caption = 'Review Threshold';
ToolTip = 'Specifies the threshold used when the agent requests a review.';
}
}
}
}
trigger OnOpenPage()
var
SalesReviewAgentSetup: Record "Sales Review Agent Setup";
begin
if IsNullGuid(Rec."User Security ID") then
exit;
if SalesReviewAgentSetup.Get(Rec."User Security ID") then
Rec := SalesReviewAgentSetup;
end;
trigger OnQueryClosePage(CloseAction: Action): Boolean
var
AgentSetup: Codeunit "Agent Setup";
AgentSetupBuffer: Record "Agent Setup Buffer";
begin
if CloseAction = CloseAction::Cancel then
exit(true);
CurrPage.AgentSetupPart.Page.GetAgentSetupBuffer(AgentSetupBuffer);
if AgentSetup.GetChangesMade(AgentSetupBuffer) then
Rec."User Security ID" := AgentSetup.SaveChanges(AgentSetupBuffer);
if IsNullGuid(Rec."User Security ID") then
exit(true);
SaveCustomProperties();
exit(true);
end;
local procedure SaveCustomProperties()
var
SalesReviewAgentSetup: Record "Sales Review Agent Setup";
begin
if not SalesReviewAgentSetup.Get(Rec."User Security ID") then begin
SalesReviewAgentSetup.Init();
SalesReviewAgentSetup."User Security ID" := Rec."User Security ID";
SalesReviewAgentSetup.Insert(true);
end;
SalesReviewAgentSetup."Review Threshold" := Rec."Review Threshold";
SalesReviewAgentSetup.Modify(true);
end;
}

30
community/knowledge/agents/agent-setup-source-table-is-temporary.md Normal file
View file

@ -0,0 +1,30 @@
---
bc-version: [27..]
domain: agents
keywords: [sourcetabletemporary, configurationdialog, savechanges, cancel, draft]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Keep the agent setup page source temporary until Update
## Description
ConfigurationDialog setup is a draft: the user can Cancel without writing. That only works if `SourceTableTemporary = true` and custom fields stay in memory until Update. Writing the real table in OnValidate or OnOpenPage commits a partial agent when the dialog errors or is cancelled.
## Best Practice
Mark the page `SourceTableTemporary = true`. Copy into the temp record on open. Persist the Agent Setup buffer and custom fields only from the close path when the action is not Cancel, using `Agent Setup.GetChangesMade` / `SaveChanges`.
See sample: `agent-setup-source-table-is-temporary.good.al`.
## Anti Pattern
A non-temporary source table, or `Insert`/`Modify` on the persisted setup row from field OnValidate. Detection signal: agent `ConfigurationDialog` without `SourceTableTemporary = true`, or database writes before Update.
See sample: `agent-setup-source-table-is-temporary.bad.al`.
## See also
`agent-setup-page-is-configuration-dialog.md` defines the setup page shape that uses this draft lifecycle.

24
community/knowledge/agents/agent-setup-table-keyed-by-user-security-id.bad.al Normal file
View file

@ -0,0 +1,24 @@
table 50100 "Sales Review Agent Setup"
{
DataClassification = CustomerContent;
fields
{
field(1; "Primary Key"; Code[10])
{
Caption = 'Primary Key';
}
field(10; "Review Threshold"; Decimal)
{
Caption = 'Review Threshold';
}
}
keys
{
key(PK; "Primary Key")
{
Clustered = true;
}
}
}

25
community/knowledge/agents/agent-setup-table-keyed-by-user-security-id.good.al Normal file
View file

@ -0,0 +1,25 @@
table 50100 "Sales Review Agent Setup"
{
DataClassification = CustomerContent;
fields
{
field(1; "User Security ID"; Guid)
{
Caption = 'User Security ID';
DataClassification = EndUserPseudonymousIdentifiers;
}
field(10; "Review Threshold"; Decimal)
{
Caption = 'Review Threshold';
}
}
keys
{
key(PK; "User Security ID")
{
Clustered = true;
}
}
}

26
community/knowledge/agents/agent-setup-table-keyed-by-user-security-id.md Normal file
View file

@ -0,0 +1,26 @@
---
bc-version: [27..]
domain: agents
keywords: [user-security-id, setup-table, primary-key, agent-instance, guid]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Agent setup tables are keyed by User Security ID
## Description
Each agent instance is a user. Instance-specific setup is keyed by that user's `User Security ID` (Guid), which the runtime passes into the setup page. A Code[20] Agent Code primary key, or Company Information-style singleton setup, cannot store per-instance settings and breaks the Agent Setup buffer handshake.
## Best Practice
Give the setup table a Guid field `User Security ID` as the clustered primary key. Other settings are attributes of that key. When the page opens, `Get` or insert by the Guid the Agent Setup part already holds.
See sample: `agent-setup-table-keyed-by-user-security-id.good.al`.
## Anti Pattern
A setup table keyed by Code, Integer, or with no Guid user key, then mapping one row to every instance. Detection signal: source table of the agent setup page whose primary key is not `User Security ID`.
See sample: `agent-setup-table-keyed-by-user-security-id.bad.al`.

8
community/knowledge/agents/analyze-message-error-stops-warning-forces-review.bad.al Normal file
View file

@ -0,0 +1,8 @@
codeunit 50100 "Sales Review Agent Task"
{
procedure AnalyzeAgentTaskMessage(AgentTaskMessage: Record "Agent Task Message"; var Annotations: Record "Agent Annotation")
begin
// No validation. Combined with SetRequiresReview(false) this auto-runs
// untrusted input. Warnings are the only way to force a review later.
end;
}

41
community/knowledge/agents/analyze-message-error-stops-warning-forces-review.good.al Normal file
View file

@ -0,0 +1,41 @@
codeunit 50100 "Sales Review Agent Task"
{
procedure AnalyzeAgentTaskMessage(AgentTaskMessage: Record "Agent Task Message"; var Annotations: Record "Agent Annotation")
var
AgentMessage: Codeunit "Agent Message";
EmptyMessageMsg: Label 'Message is empty.';
EmptyMessageDetailsTxt: Label 'Provide a sales order task before running the agent.';
NotRelevantMsg: Label 'Message is not a sales order task.';
NotRelevantDetailsTxt: Label 'Provide a message related to sales order review.';
MessageText: Text;
begin
if AgentTaskMessage.Type = AgentTaskMessage.Type::Output then begin
AgentMessage.UpdateText(AgentTaskMessage, AgentMessage.GetText(AgentTaskMessage) + #13#10 + #13#10 + 'Written with the help of AI');
exit;
end;
MessageText := AgentMessage.GetText(AgentTaskMessage);
if MessageText = '' then begin
Clear(Annotations);
Annotations.Code := 'MESSAGE001';
Annotations.Severity := Annotations.Severity::Error;
Annotations.Message := EmptyMessageMsg;
Annotations.Details := EmptyMessageDetailsTxt;
Annotations.Insert();
exit;
end;
if not IsRelevant(MessageText) then begin
Clear(Annotations);
Annotations.Code := 'RELEVANCE001';
Annotations.Severity := Annotations.Severity::Warning;
Annotations.Message := NotRelevantMsg;
Annotations.Details := NotRelevantDetailsTxt;
Annotations.Insert();
end;
end;
local procedure IsRelevant(MessageText: Text): Boolean
begin
exit(StrPos(LowerCase(MessageText), 'sales order') > 0);
end;
}

26
community/knowledge/agents/analyze-message-error-stops-warning-forces-review.md Normal file
View file

@ -0,0 +1,26 @@
---
bc-version: [27..]
domain: agents
keywords: [analyzeagenttaskmessage, agent-annotation, error, warning, setrequiresreview]
technologies: [al]
countries: [w1]
application-area: [all]
---
# AnalyzeAgentTaskMessage: Error stops the task; Warning still requires review
## Description
`IAgentTaskExecution.AnalyzeAgentTaskMessage` runs on inbound and outbound messages. An Error annotation stops processing. A Warning annotation requests user intervention. If analysis returns Warning, the platform still requires approval even when the incoming message used `SetRequiresReview(false)`. Output text can be rewritten here (signature, redaction).
## Best Practice
Validate inbound payloads in analysis: Error when the task must not run; Warning when a human must confirm. For outbound messages, adjust text in this method rather than in a later subscriber. Do not rely on skip-review to bypass warnings.
See sample: `analyze-message-error-stops-warning-forces-review.good.al`.
## Anti Pattern
Ignoring analysis entirely, or emitting Warning while documenting that `SetRequiresReview(false)` means unattended run. Detection signal: empty `AnalyzeAgentTaskMessage` plus skip-review on external input.
See sample: `analyze-message-error-stops-warning-forces-review.bad.al`.

9
community/knowledge/agents/bind-agent-subscribers-only-in-agent-session.bad.al Normal file
View file

@ -0,0 +1,9 @@
codeunit 50101 "Sales Review Agent Events"
{
[EventSubscriber(ObjectType::Table, Database::"Sales Header", OnAfterInsertEvent, '', false, false)]
local procedure OnAfterInsertSalesHeader(var Rec: Record "Sales Header")
begin
// Runs for every user session, not only the agent.
Message('Keep going, agent.');
end;
}

34
community/knowledge/agents/bind-agent-subscribers-only-in-agent-session.good.al Normal file
View file

@ -0,0 +1,34 @@
codeunit 50101 "Sales Review Agent Subscribers"
{
Access = Internal;
EventSubscriberInstance = Manual;
SingleInstance = true;
[EventSubscriber(ObjectType::Table, Database::"Sales Header", OnAfterInsertEvent, '', false, false)]
local procedure OnAfterInsertSalesHeader(var Rec: Record "Sales Header")
begin
Message('Keep going, agent.');
end;
}
codeunit 50102 "Agent Session Events"
{
Access = Internal;
SingleInstance = true;
InherentEntitlements = X;
InherentPermissions = X;
var
GlobalAgentSubscribers: Codeunit "Sales Review Agent Subscribers";
[EventSubscriber(ObjectType::Codeunit, Codeunit::"System Initialization", OnAfterInitialization, '', false, false)]
local procedure RegisterSubscribersOnAfterInitialization()
var
AgentSession: Codeunit "Agent Session";
AgentMetadataProvider: Enum "Agent Metadata Provider";
begin
if not AgentSession.IsAgentSession(AgentMetadataProvider) then
exit;
if BindSubscription(GlobalAgentSubscribers) then;
end;
}

26
community/knowledge/agents/bind-agent-subscribers-only-in-agent-session.md Normal file
View file

@ -0,0 +1,26 @@
---
bc-version: [27..]
domain: agents
keywords: [agent-session, isagentsession, bindsubscription, system-initialization, singleinstance]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Bind extra agent subscribers only inside an agent session
## Description
Page-filter tweaks, extra validation, and prompt dialogs for the agent should not run for every user. `Agent Session.IsAgentSession` distinguishes agent UI sessions. Binding those subscribers on `System Initialization` only when the session is an agent session avoids global subscriber cost. Models register `SingleInstance` table subscribers unconditionally.
## Best Practice
On `OnAfterInitialization`, exit unless `Agent Session.IsAgentSession`. Then `BindSubscription` a single-instance codeunit that holds the current task id. Keep those subscribers internal.
See sample: `bind-agent-subscribers-only-in-agent-session.good.al`.
## Anti Pattern
Event subscribers on `Sales Header` OnAfterInsert that always `Message` the agent, with no `IsAgentSession` guard. Detection signal: agent-only behaviour in a static subscriber that is not bind-gated.
See sample: `bind-agent-subscribers-only-in-agent-session.bad.al`.

10
community/knowledge/agents/cross-app-agent-calls-need-your-public-api.bad.al Normal file
View file

@ -0,0 +1,10 @@
codeunit 50110 "Other App Agent Hook"
{
procedure RenameForeignAgent(AgentUserSecurityId: Guid)
var
Agent: Codeunit Agent;
begin
// Fails at runtime when the instance was defined in another app.
Agent.SetDisplayName(AgentUserSecurityId, 'Updated Name');
end;
}

21
community/knowledge/agents/cross-app-agent-calls-need-your-public-api.good.al Normal file
View file

@ -0,0 +1,21 @@
codeunit 50110 "Sales Review Agent API"
{
Access = Public;
procedure SetDisplayName(AgentUserSecurityId: Guid; NewDisplayName: Text[80])
var
Agent: Codeunit Agent;
begin
Agent.SetDisplayName(AgentUserSecurityId, NewDisplayName);
end;
procedure SetActiveState(AgentUserSecurityId: Guid; ActivateAgent: Boolean)
var
Agent: Codeunit Agent;
begin
if ActivateAgent then
Agent.Activate(AgentUserSecurityId)
else
Agent.Deactivate(AgentUserSecurityId);
end;
}

26
community/knowledge/agents/cross-app-agent-calls-need-your-public-api.md Normal file
View file

@ -0,0 +1,26 @@
---
bc-version: [27..]
domain: agents
keywords: [cross-app, public-api, agent-create, isolation, access-public]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Other apps cannot call the toolkit APIs on your agent; publish your own API
## Description
For isolation, `Agent`, `Agent Task Builder`, and related toolkit codeunits error when the target instance belongs to another app. There is no supported way to pass another extension's metadata provider into `SetInstructions` or `Create`. Partners who need to enqueue work must call a public API you own.
## Best Practice
Expose a public codeunit in the agent app (`Access = Public`) whose procedures take `User Security ID` and forward to `Agent` / `Agent Task Builder`. Document that surface as the integration contract. Keep toolkit calls inside that app.
See sample: `cross-app-agent-calls-need-your-public-api.good.al`.
## Anti Pattern
From app B, calling `Agent.SetDisplayName` or `Agent.Create` with app A's metadata provider. Detection signal: toolkit agent APIs used with an `Agent Metadata Provider` value not declared in the same app.
See sample: `cross-app-agent-calls-need-your-public-api.bad.al`.

19
community/knowledge/agents/do-not-create-agents-in-install-upgrade-or-background.bad.al Normal file
View file

@ -0,0 +1,19 @@
codeunit 50100 "Sales Review Agent Install"
{
Subtype = Install;
trigger OnInstallAppPerCompany()
var
Agent: Codeunit Agent;
TempAgentAccessControl: Record "Agent Access Control" temporary;
AgentUserSecurityId: Guid;
begin
// Create requires an interactive session. Install is not one.
AgentUserSecurityId := Agent.Create(
Enum::"Agent Metadata Provider"::"Sales Review Agent",
'SALESREVIEW',
'Sales Review Agent',
TempAgentAccessControl);
Agent.Activate(AgentUserSecurityId);
end;
}

44
community/knowledge/agents/do-not-create-agents-in-install-upgrade-or-background.good.al Normal file
View file

@ -0,0 +1,44 @@
page 50100 "Sales Review Agent Setup"
{
PageType = ConfigurationDialog;
ApplicationArea = All;
SourceTable = "Sales Review Agent Setup";
SourceTableTemporary = true;
Extensible = false;
layout
{
area(Content)
{
part(AgentSetupPart; "Agent Setup Part")
{
ApplicationArea = All;
UpdatePropagation = Both;
}
}
}
trigger OnQueryClosePage(CloseAction: Action): Boolean
var
Agent: Codeunit Agent;
AgentSetup: Codeunit "Agent Setup";
TempAgentSetupBuffer: Record "Agent Setup Buffer" temporary;
AgentUserSecurityId: Guid;
begin
if CloseAction = CloseAction::Cancel then
exit(true);
CurrPage.AgentSetupPart.Page.GetAgentSetupBuffer(TempAgentSetupBuffer);
AgentUserSecurityId := AgentSetup.SaveChanges(TempAgentSetupBuffer);
Agent.SetInstructions(AgentUserSecurityId, GetInstructions());
Agent.Activate(AgentUserSecurityId);
exit(true);
end;
local procedure GetInstructions() Instructions: SecretText
var
InstructionsNameTxt: Label 'Instructions.txt', Locked = true;
begin
Instructions := NavApp.GetResourceAsText(InstructionsNameTxt);
end;
}

26
community/knowledge/agents/do-not-create-agents-in-install-upgrade-or-background.md Normal file
View file

@ -0,0 +1,26 @@
---
bc-version: [27..]
domain: agents
keywords: [agent-create, install, upgrade, job-queue, interactive-session, background]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Do not create agent instances from install, upgrade, or background sessions
## Description
`Agent.Create` requires an interactive user session. The platform blocks creation from install codeunits, upgrade codeunits, and background sessions (job queue, scheduled tasks). Packaging an agent in an app does not mean spinning up instances at install. Models still call `Create` from `OnInstallAppPerCompany` to activate the agent.
## Best Practice
Create instances from a setup page, a wizard, or another UI-driven path after the user is in a client session. Apply instructions and `Activate` there. For existing companies after an upgrade, document that an admin must open setup; do not create from the upgrade codeunit.
See sample: `do-not-create-agents-in-install-upgrade-or-background.good.al`.
## Anti Pattern
`Agent.Create` inside `OnInstallAppPerCompany`, `OnUpgradePerCompany`, or a job-queue codeunit. The call fails at runtime even if it compiles. Detection signal: `Agent.Create` in `Subtype = Install`, `Subtype = Upgrade`, or a non-UI session.
See sample: `do-not-create-agents-in-install-upgrade-or-background.bad.al`.

14
community/knowledge/agents/get-default-access-controls-least-privilege.bad.al Normal file
View file

@ -0,0 +1,14 @@
codeunit 50100 "Sales Review Agent Factory"
{
procedure GetDefaultAccessControls(var TempAccessControlBuffer: Record "Access Control Buffer" temporary)
var
BaseApplicationAppIdTok: Label '437dbf0e-84ff-417a-965d-ed2bb9650972', Locked = true;
begin
Clear(TempAccessControlBuffer);
TempAccessControlBuffer."Company Name" := CopyStr(CompanyName(), 1, MaxStrLen(TempAccessControlBuffer."Company Name"));
TempAccessControlBuffer.Scope := TempAccessControlBuffer.Scope::System;
TempAccessControlBuffer."App ID" := BaseApplicationAppIdTok;
TempAccessControlBuffer."Role ID" := 'D365 BUS FULL ACCESS';
TempAccessControlBuffer.Insert();
end;
}

25
community/knowledge/agents/get-default-access-controls-least-privilege.good.al Normal file
View file

@ -0,0 +1,25 @@
permissionset 50100 "SALES REVIEW AGENT"
{
Assignable = true;
Caption = 'Sales Review Agent';
Permissions =
tabledata "Sales Header" = R,
tabledata "Sales Line" = R;
}
codeunit 50100 "Sales Review Agent Factory"
{
procedure GetDefaultAccessControls(var TempAccessControlBuffer: Record "Access Control Buffer" temporary)
var
CurrentModuleInfo: ModuleInfo;
RoleIdTok: Label 'SALES REVIEW AGENT', Locked = true;
begin
NavApp.GetCurrentModuleInfo(CurrentModuleInfo);
Clear(TempAccessControlBuffer);
TempAccessControlBuffer."Company Name" := CopyStr(CompanyName(), 1, MaxStrLen(TempAccessControlBuffer."Company Name"));
TempAccessControlBuffer.Scope := TempAccessControlBuffer.Scope::System;
TempAccessControlBuffer."App ID" := CurrentModuleInfo.Id;
TempAccessControlBuffer."Role ID" := RoleIdTok;
TempAccessControlBuffer.Insert();
end;
}

30
community/knowledge/agents/get-default-access-controls-least-privilege.md Normal file
View file

@ -0,0 +1,30 @@
---
bc-version: [27..]
domain: agents
keywords: [getdefaultaccesscontrols, access-control-buffer, permissionset, least-privilege, iagentfactory]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Default agent permission sets must exist in AL and stay least privilege
## Description
`IAgentFactory.GetDefaultAccessControls` fills a temporary `Access Control Buffer` used when an instance is created. Permission sets that exist only as user-created sets in a sandbox are missing in the next environment. Granting `D365 BUS FULL ACCESS` or SUPER gives the agent a user-sized blast radius. Effective rights are still the intersection with the assigning user's permissions.
## Best Practice
Insert only the permission sets the agent needs. For an AL `permissionset` object, use `Scope::System` and the ID of the app that defines it. Recreate permission sets that exist only as user-defined configuration in Business Central as AL objects first. Prefer a dedicated permission set over a full-user role.
See sample: `get-default-access-controls-least-privilege.good.al`.
## Anti Pattern
Empty `GetDefaultAccessControls`, or inserting `SUPER` / `D365 BUS FULL ACCESS` because it made the demo work. Detection signal: Role ID on the default buffer that is a full-user role, or a set that is not in the app.
See sample: `get-default-access-controls-least-privilege.bad.al`.
## See also
`agent-permissions-intersect-with-assigner.md` explains the platform limits that still apply after default access controls are assigned.

9
community/knowledge/agents/get-default-profile-lives-in-the-app.bad.al Normal file
View file

@ -0,0 +1,9 @@
codeunit 50100 "Sales Review Agent Factory"
{
procedure GetDefaultProfile(var TempAllProfile: Record "All Profile" temporary)
begin
// Profile exists only as a user personalization in the design sandbox.
TempAllProfile."Profile ID" := 'SALES REVIEW SANDBOX';
TempAllProfile.Insert();
end;
}

20
community/knowledge/agents/get-default-profile-lives-in-the-app.good.al Normal file
View file

@ -0,0 +1,20 @@
profile "SALES REVIEW AGENT"
{
Caption = 'Sales Review Agent';
Description = 'UI surface for the Sales Review Agent.';
RoleCenter = "Order Processor Role Center";
Customizations = "Sales Review Agent Sales Ord.";
}
codeunit 50100 "Sales Review Agent Factory"
{
procedure GetDefaultProfile(var TempAllProfile: Record "All Profile" temporary)
var
Agent: Codeunit Agent;
CurrentModuleInfo: ModuleInfo;
DefaultProfileTok: Label 'SALES REVIEW AGENT', Locked = true;
begin
NavApp.GetCurrentModuleInfo(CurrentModuleInfo);
Agent.PopulateDefaultProfile(DefaultProfileTok, CurrentModuleInfo.Id, TempAllProfile);
end;
}

30
community/knowledge/agents/get-default-profile-lives-in-the-app.md Normal file
View file

@ -0,0 +1,30 @@
---
bc-version: [27..]
domain: agents
keywords: [getdefaultprofile, profile, page-customization, populatedefaultprofile, role-center]
technologies: [al]
countries: [w1]
application-area: [all]
---
# The default agent profile must be an AL profile in the app
## Description
`IAgentFactory.GetDefaultProfile` assigns the Role Center and page customizations the agent UI-navigates. A profile built only in the client, or page personalization that was never exported, is absent after deploy. `Agent.PopulateDefaultProfile` still needs a profile ID that exists in the current module.
## Best Practice
Ship a `profile` object (and page customizations) in the app. In `GetDefaultProfile`, call `Agent.PopulateDefaultProfile` with that profile ID and `NavApp.GetCurrentModuleInfo`. Include UI-exported customizations as AL.
See sample: `get-default-profile-lives-in-the-app.good.al`.
## Anti Pattern
Setting `TempAllProfile."Profile ID"` to a client-only profile, or skipping `GetDefaultProfile`. Detection signal: factory default profile ID with no matching `profile` object in the app.
See sample: `get-default-profile-lives-in-the-app.bad.al`.
## See also
`agent-profile-narrows-visible-ui.md` explains which UI the app-owned profile should expose.

9
community/knowledge/agents/instruction-structure-is-role-rules-steps.bad.al Normal file
View file

@ -0,0 +1,9 @@
codeunit 50100 "Sales Review Agent Instr."
{
procedure GetInstructions() Instructions: SecretText
var
PromptLbl: Label 'Check customer credit for the given sales order. Document the result.', Locked = true;
begin
Instructions := PromptLbl;
end;
}

17
community/knowledge/agents/instruction-structure-is-role-rules-steps.good.al Normal file
View file

@ -0,0 +1,17 @@
codeunit 50100 "Sales Review Agent Instr."
{
procedure GetInstructions() Instructions: SecretText
var
Builder: TextBuilder;
begin
Builder.AppendLine('# Responsibilities');
Builder.AppendLine('You validate sales orders against customer credit and hold status.');
Builder.AppendLine('# Guidelines');
Builder.AppendLine('Always request a review before posting or sending external mail.');
Builder.AppendLine('# Instructions');
Builder.AppendLine('1. Open the sales order named in the task.');
Builder.AppendLine('2. Check credit limit and overdue balance.');
Builder.AppendLine('3. Document the result on the order and request a review.');
Instructions := Builder.ToText();
end;
}

30
community/knowledge/agents/instruction-structure-is-role-rules-steps.md Normal file
View file

@ -0,0 +1,30 @@
---
bc-version: [27..]
domain: agents
keywords: [instructions, responsibilities, guidelines, steps, setinstructions]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Instruction documents use responsibilities, guidelines, then ordered steps
## Description
The runtime treats instructions as the agent's standing prompt. A one-line goal produces inconsistent navigation. Microsoft's instruction framework is three layers: responsibilities (what the agent owns), guidelines (rules for every task), and instructions (ordered steps per task, with substeps). That structure is BC-specific, not generic prompt flavour.
## Best Practice
Store a document that states responsibilities, then non-negotiable guidelines (when to request a review, when not to post), then numbered steps for each task. Keep that text in the resource you pass to `SetInstructions`.
See sample: `instruction-structure-is-role-rules-steps.good.al`.
## Anti Pattern
A single sentence such as Check customer credit for the sales order. Detection signal: instruction resource or `SetInstructions` payload with no responsibilities / guidelines / steps sections.
See sample: `instruction-structure-is-role-rules-steps.bad.al`.
## See also
`instructions-describe-work-not-tool-ids.md` and `use-documented-instruction-keywords.md` define how to write the steps inside this structure.

7
community/knowledge/agents/instructions-describe-work-not-tool-ids.bad.al Normal file
View file

@ -0,0 +1,7 @@
codeunit 50100 "Sales Review Agent Instr."
{
procedure GetInstructions() Instructions: SecretText
begin
Instructions := 'Open page 42. Invoke action Post_Promoted. Use tool SalesOrder.CreditCheck_v3.';
end;
}

12
community/knowledge/agents/instructions-describe-work-not-tool-ids.good.al Normal file
View file

@ -0,0 +1,12 @@
codeunit 50100 "Sales Review Agent Instr."
{
procedure GetInstructions() Instructions: SecretText
var
Builder: TextBuilder;
begin
Builder.AppendLine('Memorize the sales order number from the task.');
Builder.AppendLine('Set the order on hold when credit fails, with a reason.');
Builder.AppendLine('When credit passes, request a review before posting the order.');
Instructions := Builder.ToText();
end;
}

30
community/knowledge/agents/instructions-describe-work-not-tool-ids.md Normal file
View file

@ -0,0 +1,30 @@
---
bc-version: [27..]
domain: agents
keywords: [instructions, tools, invoke-action, memorize, page-actions]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Instructions describe outcomes, not page action or tool names
## Description
Agent tools are the UI the profile exposes. Action names and tool ids change across pages and versions. Best-practice guidance is to say what to accomplish, not which tool to invoke. Page state is also not fully in history; values needed later must be memorized. Models paste Promoted action names into the prompt.
## Best Practice
Write steps as business outcomes (release the order, set the hold reason). Tell the agent to memorize identifiers it must reuse. Do not hard-code action captions or tool ids.
See sample: `instructions-describe-work-not-tool-ids.good.al`.
## Anti Pattern
Instructions that say invoke SalesOrder.Post_Promoted or use tool page-42-action-3. Detection signal: instruction text containing Promoted action names or tool identifiers.
See sample: `instructions-describe-work-not-tool-ids.bad.al`.
## See also
`instruction-structure-is-role-rules-steps.md` defines the containing document structure, and `use-documented-instruction-keywords.md` identifies runtime-recognized phrases.

18
community/knowledge/agents/reapply-resource-instructions-on-upgrade.bad.al Normal file
View file

@ -0,0 +1,18 @@
codeunit 50100 "Sales Review Agent Create"
{
procedure CreateWithInstructions()
var
Agent: Codeunit Agent;
TempAgentAccessControl: Record "Agent Access Control" temporary;
AgentUserSecurityId: Guid;
InstructionsNameTxt: Label 'Instructions.txt', Locked = true;
begin
AgentUserSecurityId := Agent.Create(
Enum::"Agent Metadata Provider"::"Sales Review Agent",
'SALESREVIEW',
'Sales Review Agent',
TempAgentAccessControl);
// Only new instances get the resource. Upgrades never re-apply it.
Agent.SetInstructions(AgentUserSecurityId, NavApp.GetResourceAsText(InstructionsNameTxt));
end;
}

33
community/knowledge/agents/reapply-resource-instructions-on-upgrade.good.al Normal file
View file

@ -0,0 +1,33 @@
codeunit 50100 "Sales Review Agent Upgrade"
{
Subtype = Upgrade;
trigger OnUpgradePerCompany()
var
Agent: Codeunit Agent;
UpgradeTag: Codeunit "Upgrade Tag";
Instructions: SecretText;
AgentUserSecurityIds: List of [Guid];
AgentUserSecurityId: Guid;
TagTxt: Label 'SALESREVIEW-INSTR-2.0.0', Locked = true;
InstructionsNameTxt: Label 'Instructions.txt', Locked = true;
begin
if UpgradeTag.HasUpgradeTag(TagTxt) then
exit;
Instructions := NavApp.GetResourceAsText(InstructionsNameTxt);
AgentUserSecurityIds := GetExistingAgentUserIds();
foreach AgentUserSecurityId in AgentUserSecurityIds do
Agent.SetInstructions(AgentUserSecurityId, Instructions);
UpgradeTag.SetUpgradeTag(TagTxt);
end;
local procedure GetExistingAgentUserIds() AgentUserSecurityIds: List of [Guid]
var
SalesReviewAgentSetup: Record "Sales Review Agent Setup";
begin
if SalesReviewAgentSetup.FindSet() then
repeat
AgentUserSecurityIds.Add(SalesReviewAgentSetup."User Security ID");
until SalesReviewAgentSetup.Next() = 0;
end;
}

26
community/knowledge/agents/reapply-resource-instructions-on-upgrade.md Normal file
View file

@ -0,0 +1,26 @@
---
bc-version: [27..]
domain: agents
keywords: [upgrade, setinstructions, navapp-getresourceastext, existing-instances, upgrade-tag]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Reapply resource instructions to existing agent instances on upgrade
## Description
Static instructions stored as an app resource are copied onto an instance only when you call `SetInstructions`. Shipping a new `Instructions.txt` in version 2.0 does not update agents created under 1.0. Models change the resource and assume running instances pick it up.
## Best Practice
In the upgrade codeunit, find existing instances of your metadata provider and call `SetInstructions` again with `NavApp.GetResourceAsText`. Guard with an upgrade tag so the rewrite runs once per version that changes the file.
See sample: `reapply-resource-instructions-on-upgrade.good.al`.
## Anti Pattern
Editing only the resource file, or calling `SetInstructions` solely from the first-time setup path. Detection signal: instruction resource in `resourceFolders` with no upgrade procedure that re-applies it.
See sample: `reapply-resource-instructions-on-upgrade.bad.al`.

21
community/knowledge/agents/register-copilot-capability-for-the-agent.bad.al Normal file
View file

@ -0,0 +1,21 @@
enumextension 50100 "Sales Review Agent Metadata" extends "Agent Metadata Provider"
{
value(50100; "Sales Review Agent")
{
Caption = 'Sales Review Agent';
Implementation = IAgentFactory = "Sales Review Agent Factory",
IAgentMetadata = "Sales Review Agent Metadata",
IAgentTaskExecution = "Sales Review Agent Task";
}
}
codeunit 50101 "Sales Review Agent Install"
{
Subtype = Install;
Access = Internal;
trigger OnInstallAppPerDatabase()
begin
// Agent type exists, but no Copilot Capability value and no RegisterCapability.
end;
}

26
community/knowledge/agents/register-copilot-capability-for-the-agent.good.al Normal file
View file

@ -0,0 +1,26 @@
enumextension 50101 "Sales Review Agent Copilot" extends "Copilot Capability"
{
value(50101; "Sales Review Agent")
{
Caption = 'Sales Review Agent';
}
}
codeunit 50101 "Sales Review Agent Install"
{
Subtype = Install;
Access = Internal;
trigger OnInstallAppPerDatabase()
var
CopilotCapability: Codeunit "Copilot Capability";
LearnMoreUrlTxt: Label 'https://example.com/sales-review-agent', Locked = true;
begin
if not CopilotCapability.IsCapabilityRegistered(Enum::"Copilot Capability"::"Sales Review Agent") then
CopilotCapability.RegisterCapability(
Enum::"Copilot Capability"::"Sales Review Agent",
Enum::"Copilot Availability"::Preview,
Enum::"Copilot Billing Type"::"Microsoft Billed",
LearnMoreUrlTxt);
end;
}

30
community/knowledge/agents/register-copilot-capability-for-the-agent.md Normal file
View file

@ -0,0 +1,30 @@
---
bc-version: [27..]
domain: agents
keywords: [copilot-capability, registercapability, install, feature-switch, enumextension]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Register a Copilot capability for the agent on install
## Description
Each agent type needs a `Copilot Capability` enum value that the factory links as the feature switch and billing surface. The capability is invisible on Copilot and agent capabilities until an install codeunit calls `RegisterCapability` when it is not already registered. Unique ordinals matter across installed apps. Models often extend `Agent Metadata Provider` and never register the capability.
## Best Practice
Extend `Copilot Capability` with a unique value. In `OnInstallAppPerDatabase`, call `Copilot Capability.IsCapabilityRegistered` and, if false, `RegisterCapability` with availability, billing type, and a learn-more URL. Point `IAgentFactory` at that capability.
See sample: `register-copilot-capability-for-the-agent.good.al`.
## Anti Pattern
Shipping the agent enum without a `Copilot Capability` value, or adding the enum but never calling `RegisterCapability`. Duplicate ordinals across extensions also collide. Detection signal: agent metadata provider with no matching capability registration in an install codeunit.
See sample: `register-copilot-capability-for-the-agent.bad.al`.
## See also
`wire-all-three-agent-interfaces.md` covers registration of the provider implementation that references this capability.

19
community/knowledge/agents/set-instructions-as-secrettext.bad.al Normal file
View file

@ -0,0 +1,19 @@
codeunit 50100 "Sales Review Agent Create"
{
procedure CreateWithInstructions()
var
Agent: Codeunit Agent;
TempAgentAccessControl: Record "Agent Access Control" temporary;
AgentUserSecurityId: Guid;
InstructionsLbl: Label 'You are a sales validation agent. Check credit.', Locked = true;
begin
AgentUserSecurityId := Agent.Create(
Enum::"Agent Metadata Provider"::"Sales Review Agent",
'SALESREVIEW',
'Sales Review Agent',
TempAgentAccessControl);
// Label/text is not SecretText and is type-wide, not per instance.
Agent.SetInstructions(AgentUserSecurityId, InstructionsLbl);
Agent.Activate(AgentUserSecurityId);
end;
}

20
community/knowledge/agents/set-instructions-as-secrettext.good.al Normal file
View file

@ -0,0 +1,20 @@
codeunit 50100 "Sales Review Agent Create"
{
procedure CreateWithInstructions()
var
Agent: Codeunit Agent;
TempAgentAccessControl: Record "Agent Access Control" temporary;
AgentUserSecurityId: Guid;
Instructions: SecretText;
InstructionsNameTxt: Label 'Instructions.txt', Locked = true;
begin
AgentUserSecurityId := Agent.Create(
Enum::"Agent Metadata Provider"::"Sales Review Agent",
'SALESREVIEW',
'Sales Review Agent',
TempAgentAccessControl);
Instructions := NavApp.GetResourceAsText(InstructionsNameTxt);
Agent.SetInstructions(AgentUserSecurityId, Instructions);
Agent.Activate(AgentUserSecurityId);
end;
}

26
community/knowledge/agents/set-instructions-as-secrettext.md Normal file
View file

@ -0,0 +1,26 @@
---
bc-version: [27..]
domain: agents
keywords: [setinstructions, secrettext, instructions, per-instance, resource]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Set agent instructions as SecretText on the instance
## Description
Instructions are instance data, not an enum caption. `Agent.SetInstructions` takes `SecretText` so the payload is not logged or copied as ordinary text. A Label or plaintext Text on the agent type is the wrong store: it leaks into telemetry-friendly strings and cannot vary per instance or company.
## Best Practice
Load instruction text from a resource or builder into a `SecretText` variable and call `Agent.SetInstructions(AgentUserSecurityId, Instructions)` after `Create`. Keep one instruction document per instance.
See sample: `set-instructions-as-secrettext.good.al`.
## Anti Pattern
Passing a `Label` or `Text` to `SetInstructions`, storing instructions in a setup Text field without wrapping as `SecretText`, or putting the prompt only in a code comment. Detection signal: `SetInstructions` with a non-`SecretText` argument, or no `SetInstructions` after `Create`.
See sample: `set-instructions-as-secrettext.bad.al`.

36
community/knowledge/agents/show-can-create-agent-does-not-block-code-create.bad.al Normal file
View file

@ -0,0 +1,36 @@
codeunit 50100 "Sales Review Agent Factory"
{
procedure ShowCanCreateAgent(): Boolean
begin
// Author intends this to forbid all creates. It only hides the UI tile.
exit(false);
end;
}
pageextension 50100 "Sales Order List Agent Create" extends "Sales Order List"
{
actions
{
addlast(Processing)
{
action(CreateAgent)
{
ApplicationArea = All;
Caption = 'Create review agent';
trigger OnAction()
var
Agent: Codeunit Agent;
TempAgentAccessControl: Record "Agent Access Control" temporary;
begin
// Still succeeds for any caller with permission to run this action.
Agent.Create(
Enum::"Agent Metadata Provider"::"Sales Review Agent",
'SALESREVIEW',
'Sales Review Agent',
TempAgentAccessControl);
end;
}
}
}
}

25
community/knowledge/agents/show-can-create-agent-does-not-block-code-create.good.al Normal file
View file

@ -0,0 +1,25 @@
codeunit 50100 "Sales Review Agent Factory"
{
procedure ShowCanCreateAgent(): Boolean
var
AgentSystemPermissions: Codeunit "Agent System Permissions";
begin
// Hides the type from non-admins in the UI. Does not block Agent.Create.
exit(AgentSystemPermissions.CurrentUserHasCanManageAllAgentsPermission());
end;
procedure CreateIfAllowed()
var
Agent: Codeunit Agent;
AgentSystemPermissions: Codeunit "Agent System Permissions";
TempAgentAccessControl: Record "Agent Access Control" temporary;
begin
if not AgentSystemPermissions.CurrentUserHasCanManageAllAgentsPermission() then
Error('Only agent administrators can create this agent.');
Agent.Create(
Enum::"Agent Metadata Provider"::"Sales Review Agent",
'SALESREVIEW',
'Sales Review Agent',
TempAgentAccessControl);
end;
}

26
community/knowledge/agents/show-can-create-agent-does-not-block-code-create.md Normal file
View file

@ -0,0 +1,26 @@
---
bc-version: [28..]
domain: agents
keywords: [showcancreateagent, agent-discovery, agent-create, administrator, agent-configuration-rights]
technologies: [al]
countries: [w1]
application-area: [all]
---
# ShowCanCreateAgent only hides UI create, not programmatic create
## Description
`IAgentFactory.ShowCanCreateAgent` controls whether the type appears in the in-client create UI. Returning false does not stop `Agent.Create` from AL. From 28.1, non-admins can discover extension agents unless this method (and agent configuration rights) restrict them. Models treat a false return as a hard create lock.
## Best Practice
Use `ShowCanCreateAgent` to decide discovery. If only agent administrators should see the type, return `Agent System Permissions.CurrentUserHasCanManageAllAgentsPermission`. Enforce extra policy inside your own create API. Never assume UI hiding blocks code.
See sample: `show-can-create-agent-does-not-block-code-create.good.al`.
## Anti Pattern
Returning `exit(false)` from `ShowCanCreateAgent` and then documenting that instances cannot be created, while page actions or other apps still call `Agent.Create`. Detection signal: `ShowCanCreateAgent` always false with no matching guard on programmatic create.
See sample: `show-can-create-agent-does-not-block-code-create.bad.al`.

15
community/knowledge/agents/skip-incoming-review-only-for-trusted-input.bad.al Normal file
View file

@ -0,0 +1,15 @@
codeunit 50100 "Sales Review Agent Tasks"
{
procedure EnqueueFromEmailBody(RawEmailBody: Text; AgentUserSecurityId: Guid)
var
AgentTaskBuilder: Codeunit "Agent Task Builder";
AgentTaskMessageBuilder: Codeunit "Agent Task Message Builder";
AgentTask: Record "Agent Task";
begin
AgentTaskMessageBuilder.Initialize('Internet', RawEmailBody)
.SetRequiresReview(false);
AgentTask := AgentTaskBuilder.Initialize(AgentUserSecurityId, 'Process inbound mail')
.AddTaskMessage(AgentTaskMessageBuilder)
.Create();
end;
}

16
community/knowledge/agents/skip-incoming-review-only-for-trusted-input.good.al Normal file
View file

@ -0,0 +1,16 @@
codeunit 50100 "Sales Review Agent Tasks"
{
procedure EnqueueFromSalesOrder(SalesHeader: Record "Sales Header"; AgentUserSecurityId: Guid)
var
AgentTaskBuilder: Codeunit "Agent Task Builder";
AgentTaskMessageBuilder: Codeunit "Agent Task Message Builder";
AgentTask: Record "Agent Task";
begin
SalesHeader.TestField("No.");
AgentTaskMessageBuilder.Initialize('Sales Team', 'Review sales order ' + SalesHeader."No.")
.SetRequiresReview(false);
AgentTask := AgentTaskBuilder.Initialize(AgentUserSecurityId, 'Review Sales Order')
.AddTaskMessage(AgentTaskMessageBuilder)
.Create();
end;
}

26
community/knowledge/agents/skip-incoming-review-only-for-trusted-input.md Normal file
View file

@ -0,0 +1,26 @@
---
bc-version: [28..]
domain: agents
keywords: [setrequiresreview, agent-task-message-builder, approval, trusted-input, skip-review]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Skip incoming message review only after the caller validated the payload
## Description
Incoming task messages default to requiring user approval before the agent runs. From 28.1, `Agent Task Message Builder.SetRequiresReview(false)` starts the agent immediately. That is safe only for inputs you already validated in AL (your page action, your posting subscriber). External email or partner payloads are not trusted by default. Analysis Warnings still force a review.
## Best Practice
Leave the default review-on for anything that originated outside your extension. Call `SetRequiresReview(false)` only on messages you constructed from already-authorized BC data.
See sample: `skip-incoming-review-only-for-trusted-input.good.al`.
## Anti Pattern
`SetRequiresReview(false)` on simulated email, incoming webhooks, or user-free text. Detection signal: `SetRequiresReview(false)` next to external content with no prior validation.
See sample: `skip-incoming-review-only-for-trusted-input.bad.al`.

7
community/knowledge/agents/use-documented-instruction-keywords.bad.al Normal file
View file

@ -0,0 +1,7 @@
codeunit 50100 "Sales Review Agent Instr."
{
procedure GetInstructions() Instructions: SecretText
begin
Instructions := 'When done, email the customer and remember the credit limit. Click Post_Promoted.';
end;
}

13
community/knowledge/agents/use-documented-instruction-keywords.good.al Normal file
View file

@ -0,0 +1,13 @@
codeunit 50100 "Sales Review Agent Instr."
{
procedure GetInstructions() Instructions: SecretText
var
Builder: TextBuilder;
begin
Builder.AppendLine('When the sales order is ready, request a review before posting.');
Builder.AppendLine('If a field is missing, ask for assistance.');
Builder.AppendLine('Memorize the customer credit limit for later steps.');
Builder.AppendLine('When confirmed, write an email to the salesperson; outbound mail is reviewed.');
Instructions := Builder.ToText();
end;
}

30
community/knowledge/agents/use-documented-instruction-keywords.md Normal file
View file

@ -0,0 +1,30 @@
---
bc-version: [27..]
domain: agents
keywords: [instruction-keywords, request-a-review, memorize, write-an-email, invoke-action]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Use the toolkit instruction keywords for review, mail, and memory
## Description
The agent runtime looks for specific phrases: ask for assistance, request a review, reply, write an email, memorize, `Set field`, use lookup, `Invoke action`. Ordinary English such as get a human to look or remember this is weaker. Outbound reply and email always require review; that is platform policy, not optional tone.
## Best Practice
In the instruction resource, use those keywords at the decision points: request a review before posting; write an email only after stating that outbound mail is reviewed; memorize values the later steps need. Pair `Reply` / `Write an email` with an explicit review sentence.
See sample: `use-documented-instruction-keywords.good.al`.
## Anti Pattern
Inventing tool-like verbs (call Copilot, click Post_Promoted) or omitting request a review before posting. Detection signal: instruction text that says email the customer with no review keyword.
See sample: `use-documented-instruction-keywords.bad.al`.
## See also
`instruction-structure-is-role-rules-steps.md` defines the containing document structure, while `instructions-describe-work-not-tool-ids.md` keeps outcomes independent of UI tool identifiers.

9
community/knowledge/agents/wire-all-three-agent-interfaces.bad.al Normal file
View file

@ -0,0 +1,9 @@
enumextension 50100 "Sales Review Agent Metadata" extends "Agent Metadata Provider"
{
value(50100; "Sales Review Agent")
{
Caption = 'Sales Review Agent';
// Only factory is bound. Metadata UI and task execution never resolve.
Implementation = IAgentFactory = "Sales Review Agent Factory";
}
}

10
community/knowledge/agents/wire-all-three-agent-interfaces.good.al Normal file
View file

@ -0,0 +1,10 @@
enumextension 50100 "Sales Review Agent Metadata" extends "Agent Metadata Provider"
{
value(50100; "Sales Review Agent")
{
Caption = 'Sales Review Agent';
Implementation = IAgentFactory = "Sales Review Agent Factory",
IAgentMetadata = "Sales Review Agent Meta. Impl.",
IAgentTaskExecution = "Sales Review Agent Task";
}
}

30
community/knowledge/agents/wire-all-three-agent-interfaces.md Normal file
View file

@ -0,0 +1,30 @@
---
bc-version: [27..]
domain: agents
keywords: [agent-metadata-provider, iagentfactory, iagentmetadata, iagenttaskexecution, enumextension, implementation]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Wire all three agent interfaces on the metadata provider
## Description
An AL agent type is registered by extending `Agent Metadata Provider`. The platform locates factory, metadata, and task-execution behaviour only through the `Implementation` property on that enum value. Omitting `IAgentFactory`, `IAgentMetadata`, or `IAgentTaskExecution` leaves create, UI identity, or task runs unbound. Models often ship a single codeunit and skip the enum wiring.
## Best Practice
On the enum value, set `Implementation` for all three interfaces, each pointing at a dedicated codeunit. Keep factory (create, defaults, first-time setup), metadata (setup page, summary, annotations), and task execution (message analysis, intervention suggestions) in separate objects.
See sample: `wire-all-three-agent-interfaces.good.al`.
## Anti Pattern
An `Agent Metadata Provider` value with no `Implementation`, only one interface mapped, or all three interfaces pointing at one catch-all codeunit that cannot satisfy the contracts. Detection signal: enumextension of `Agent Metadata Provider` whose value does not list `IAgentFactory`, `IAgentMetadata`, and `IAgentTaskExecution`.
See sample: `wire-all-three-agent-interfaces.bad.al`.
## See also
`register-copilot-capability-for-the-agent.md` covers the feature capability linked by the factory implementation.

70
community/skills/review/al-agents-review.md Normal file
View file

@ -0,0 +1,70 @@
---
kind: action-skill
id: al-agents-review
version: 1
title: AL agents review
description: Reviews AL source changes against agent guidance from BCQuality.
inputs: [pr-diff, file-path]
outputs: [findings-report]
bc-version: [all]
technologies: [al]
countries: [w1]
application-area: [all]
---
# AL agents review
Reviews AL source changes against the `agents` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is not one of the skills composed by `al-code-review`; Entry discovers and dispatches it as a top-level peer, so it produces an independent findings report.
Agent findings apply to AL files that implement or invoke Agent SDK surfaces, including agent interfaces, setup, creation, task execution, capability registration, profiles, access controls, instructions, and session-bound subscribers. Return `not-applicable` when the diff contains no AL changes or no Agent SDK implementation or usage.
An orchestrator invokes this skill with either a `pr-diff` or a `file-path`. The skill produces one JSON document conforming to the DO output contract.
## Source
Read the root `knowledge-index.json` generated by Entry and select entries whose `domain` is `agents` across every enabled layer. Use index metadata for candidate selection and open an article body only after it enters the worklist.
## Relevance
Apply READ's frontmatter matching semantics to the target BC version, AL technology, countries, and application areas. If a dimension is unknown, retain conditionally applicable guidance only when configuration permits it; cap resulting confidence at `medium` and name the unknown dimension in the finding message.
## Worklist
Match changed objects, procedures, interfaces, and tokens against article keywords, titles, descriptions, and paths. Give particular weight to:
- Implementations of `IAgentFactory`, `IAgentMetadata`, and `IAgentTaskExecution`.
- Agent setup tables and `ConfigurationDialog` pages using `Agent Setup`, `Agent Setup Buffer`, or `Agent Setup Part`.
- Agent creation, upgrade, capability registration, profile configuration, access controls, and subscriber binding.
- Instruction construction, `SecretText`, documented instruction keywords, task messages, trusted input, warnings, errors, and review behavior.
- Public APIs invoked by agent tasks across app boundaries.
Use these targeted rules to avoid broad token-only matches:
- Worklist setup-page shape guidance when the page returned by agent metadata is not a `ConfigurationDialog` or omits `Agent Setup Part`.
- Worklist temporary-source guidance when setup writes occur before a non-Cancel close path or a setup page is not temporary.
- Worklist permission guidance when default access controls are broad or when code assumes an agent can exceed the assigning user's permissions.
- Worklist instruction guidance only for text used as agent instructions; do not flag unrelated prompts, labels, or user-facing help.
- Worklist session-binding guidance only when subscribers are bound outside an agent session or left bound after execution.
After selection, resolve conflicting guidance using READ's layer precedence. Record displaced candidates in `suppressed` with `reason: "layer-precedence"`; record disabled-layer candidates with `reason: "configuration"`.
An empty worklist caused by absent applicable knowledge produces `no-knowledge`. An empty worklist caused by no match produces `completed` with no findings.
## Action
Evaluate each worklisted article's `## Best Practice` and `## Anti Pattern` against the changed code:
- Emit `major` for a clear anti-pattern and `minor` for a concrete best-practice contradiction.
- Use `blocker` only when the article identifies a violated platform guarantee.
- Do not emit a finding from applicability alone.
- Set confidence to `high` for unambiguous syntax or identifier evidence, `medium` for heuristic or conditionally applicable evidence, and `low` only for an explicit advisory.
Agent-originated findings without a matching article must follow the DO contract: prefix the ID with `agent:`, use `references: []`, cap severity at `minor` and confidence at `medium`, and emit only concrete defects within the agents domain.
Provide `suggested-code` when the repair is small, local, and unambiguous. Otherwise, when a mechanical-looking repair depends on missing context or has multiple valid forms, set `suggested-code-omission-reason`.
Use the standard DO outcomes: `completed`, `no-knowledge`, `not-applicable`, `partial`, or `failed`.
## Output
Return only one JSON document conforming to the DO output contract. Every finding emitted by this skill MUST set `findings[].domain` to `"Agents"`. Knowledge-backed finding IDs and references MUST use the exact repository-relative article path from the knowledge index.

6
evaluation/README.md
View file

@ -1,6 +1,6 @@
# AL review evaluation
The evaluation is convention-driven. For every `microsoft/skills/review/al-<domain>-review.md` leaf, the harness finds `microsoft/knowledge/<domain>/`, selects the first article (by filename) with both `.bad.al` and `.good.al` companions, and derives the expected positive and clean control automatically. Adding a conforming leaf requires no scoring-contract edit.
The evaluation is convention-driven. The harness discovers every `<layer>/skills/review/al-<domain>-review.md` leaf across the enabled `microsoft`, `community`, and `custom` layers. Duplicate domains resolve with `custom > community > microsoft` precedence. For each selected leaf, the harness finds paired knowledge across the same layers, applies the same precedence to duplicate article slugs, selects the first article (by filename) with both `.bad.al` and `.good.al` companions, and derives the expected positive and clean control automatically. Adding a conforming leaf requires no scoring-contract edit.
`review-fixtures.json` contains only global thresholds and optional exceptional overrides. An override may select a different article or add context when the generic convention cannot express a scenario. It should remain empty in the normal case.
@ -12,7 +12,7 @@ Model-facing preparation hashes case IDs, neutralizes `Good`/`Bad` object-name t
pwsh ./tools/Test-ReviewFixtures.ps1 -Root .
```
This credential-free check proves every registered leaf maps to a same-named knowledge domain with at least one complete AL sample pair and that all configured overrides are valid.
This credential-free check proves every selected leaf maps to a same-named knowledge domain with at least one complete AL sample pair and that all configured overrides are valid.
## Run a fast-model evaluation
@ -26,7 +26,7 @@ This credential-free check proves every registered leaf maps to a same-named kno
2. For a fast/small model, use one fresh invocation per `request-case-*.json`. Each request embeds the exact leaf instructions, that domain's candidate index rows with authoritative paths, and one opaque case. The model opens only matching articles and copies finding IDs from `candidateArticles[].path`. Save each response with the matching `result-case-*.json` name in the same directory.
`request-<domain>.json` files provide optional two-case leaf batches; save those as `result-<domain>.json`. Directory scoring prefers `result-case-*.json` when present and otherwise falls back to `result-*.json`. `review-request.json` is an optional all-domains stress test for larger models. Neither batch form is the preferred fast-model profile.
`request-<domain>.json` files provide optional two-case leaf batches and identify the selected layer-owned skill path; save those as `result-<domain>.json`. Directory scoring prefers `result-case-*.json` when present and otherwise falls back to `result-*.json`. `review-request.json` is an optional all-domains stress test for larger models. Neither batch form is the preferred fast-model profile.
3. Save only this result shape:

8
evaluation/review-fixtures.json
View file

@ -4,6 +4,9 @@
"minimumExpectedRecall": 1.0,
"minimumCleanRate": 1.0,
"overrides": {
"agents": {
"article": "wire-all-three-agent-interfaces"
},
"appsource": {
"context": "AppSourceCop mandatoryAffixes is configured to ABC."
},
@ -11,7 +14,7 @@
"article": "do-not-expose-sensitive-data-through-public-api"
},
"events": {
"article": "initialize-ishandled-to-false-before-publishing"
"article": "reset-ishandled-only-when-the-value-can-carry-over"
},
"interfaces": {
"article": "set-defaultimplementation-on-enum"
@ -28,6 +31,9 @@
"telemetry": {
"article": "telemetry-event-id-stable-unique"
},
"testing": {
"article": "ui-handlers-in-tests"
},
"upgrade": {
"article": "initvalue-does-not-update-existing-rows",
"context": "The extended table existed in the previous app version and already contains rows."

4
microsoft/knowledge/appsource/object-affixes-prevent-collisions.md
View file

@ -1,7 +1,7 @@
---
bc-version: [all]
domain: appsource
keywords: [object-affix, prefix, suffix, as0011, appsourcecop, collision, tableextension]
keywords: [object-affix, prefix, suffix, as0011, appsourcecop, collision, tableextension, first-party, isv]
technologies: [al]
countries: [w1]
application-area: [all]
@ -15,6 +15,8 @@ An AppSource extension must prevent name collisions through its registered affix
AppSourceCop enforces this. The primary rule is AS0011 ("An affix is required"); the affixes are configured through `mandatoryAffixes` (and `mandatoryPrefix`) in `AppSourceCop.json`. Two placements matter and are easy to get half-right: an object you define carries the affix at **object-name** level, while a member you add to a **standard** object carries the affix on that **member's** name. Adding an affixed object is not enough — an unaffixed field bolted onto `Customer` still collides and still fails validation.
This rule scopes to Marketplace ISV extensions, which is what AppSourceCop validates. A first-party Microsoft in-box module (publisher `Microsoft`, an object range reserved for first-party use, and no `AppSourceCop.json`/`mandatoryAffixes` in the app) is not built or shipped as an Marketplace extension and is not subject to AS0011, so an unaffixed action or field it adds to a base-application page is not a collision risk to flag. Renaming an existing shipped first-party member to add an affix is itself a breaking change to that module's own history and is not required by this rule.
## Best Practice
Own objects use the registered affix (for example `ABC Loyalty Tier`) or, when targeting BC23 or later, a qualifying namespace. Every field or action added to a standard object remains individually affixed (for example `Loyalty Points ABC` on a `Customer` tableextension).

8
microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.md
View file

@ -1,7 +1,7 @@
---
bc-version: [23..]
domain: appsource
keywords: [namespace, two-level, affix, prefix, suffix, as0011, tableextension, pageextension]
keywords: [namespace, two-level, affix, prefix, suffix, as0011, tableextension, pageextension, false-positive]
technologies: [al]
countries: [w1]
application-area: [all]
@ -13,14 +13,16 @@ application-area: [all]
Current AppSource naming guidance accepts a namespace with at least two levels, such as `Contoso.Rentals`, instead of a registered prefix or suffix on the names of objects the app owns. The namespace does not qualify members added to another publisher's object: fields, keys, controls, and actions introduced through table or page extensions still share the target object's flat member namespace and still need the registered affix.
The requirement comes from AppSourceCop rule AS0011, which only runs when the app enables AppSourceCop and configures a mandatory affix — normally an `AppSourceCop.json` next to the app manifest. An app that ships no such configuration is not subject to AS0011, and its extension members are not a compliance gap. This is the usual situation for first-party, in-box apps that ship as part of the product rather than through AppSource: their uniqueness comes from allocated object ID ranges and a controlled source tree, not from a registered affix. Confirm the extending app actually configures a mandatory affix before reporting an unaffixed extension member.
## Best Practice
Choose one collision strategy for owned objects: a registered affix or a globally meaningful namespace with at least two levels. Regardless of that choice, apply the registered affix to every member added to a base or third-party object. Keep the affix configured for AppSourceCop so member validation remains deterministic.
Choose one collision strategy for owned objects: a registered affix or a globally meaningful namespace with at least two levels. Regardless of that choice, apply the registered affix to every member added to a base or third-party object. Keep the affix configured for AppSourceCop so member validation remains deterministic. Do not raise a missing member affix against an app that does not enable AppSourceCop with a mandatory affix; there AS0011 never fires, and the app's namespace is not the reason — the absent configuration is.
See sample: `two-level-namespace-replaces-object-affix-not-extension-member-affix.good.al`.
## Anti Pattern
Using `namespace Contoso;` as though one level satisfied the AppSource alternative, or declaring `namespace Contoso.Rentals;` and then adding an unaffixed `Loyalty Points` field to `Customer`. The namespace distinguishes the extension's own objects; it cannot disambiguate members on Customer.
Using `namespace Contoso;` as though one level satisfied the AppSource alternative, or declaring `namespace Contoso.Rentals;` and then adding an unaffixed `Loyalty Points` field to `Customer` in an app that does configure a mandatory affix. The namespace distinguishes the extension's own objects; it cannot disambiguate members on Customer. The mirror-image mistake is reporting an unaffixed extension member in an app that enables no mandatory affix at all — AS0011 does not apply there, and the finding is a false positive.
See sample: `two-level-namespace-replaces-object-affix-not-extension-member-affix.bad.al`.

6
microsoft/knowledge/breaking-changes/do-not-change-published-procedure-signatures.md
View file

@ -1,7 +1,7 @@
---
bc-version: [all]
domain: breaking-changes
keywords: [signature, public-procedure, parameter, return-value, overload, contract]
keywords: [signature, public-procedure, parameter, return-value, overload, contract, integration-event]
technologies: [al]
countries: [w1]
application-area: [all]
@ -13,6 +13,8 @@ application-area: [all]
A procedure that is reachable from outside its object — any procedure not marked `local` (and, for on-prem-scoped code, anything a dependent app can still bind to) — is a contract. Once another extension compiles against it, changing its shape breaks that extension at build time. Signature changes include adding, removing, or reordering parameters, changing a parameter or return type, and toggling a parameter between by-value and `var` (by-reference). The platform treats the procedure's identity as its full signature, so even a "compatible-looking" tweak is a new method to dependents. There is exactly one safe edit: naming a previously unnamed return value, which adds no caller obligation. LLMs routinely "improve" a public procedure in place by adding a parameter, not realizing every consumer must be recompiled.
This rule governs procedures that dependents *call*. An event publisher — a procedure carrying `[IntegrationEvent]` or `[BusinessEvent]`, conventionally declared `local` — is bound to, not called, and AL binds each subscriber parameter by name rather than by position. Adding a parameter to a shipped event therefore leaves every existing subscriber binding successfully, at any position in the list, so it is additive rather than breaking and must not be flagged under this rule. See `events/adding-a-parameter-to-an-event-is-not-a-breaking-change` for the full treatment, and `events/add-new-event-parameters-at-the-end` for when publisher access does make the addition breaking. Every other edit to a published event signature — removing or retyping a parameter, renaming one, or flipping one to or from `var` — still breaks subscribers, and `events/treat-local-and-internal-events-as-subscriber-contracts` owns that case together with the analyzer rules that enforce it: AS0025 for parameter names and types, AS0063 for removing `var`, and AS0077 for adding it. Adding `var IsHandled: Boolean` is a separate concern: it binds fine but changes the event's contract, and is covered by `events/do-not-add-ishandled-to-an-existing-event`.
## Best Practice
Treat a published signature as frozen. When new behavior needs more inputs, add a new procedure or overload alongside the original — for example a `CalculateDiscountWithRate(Amount; Rate)` next to the unchanged `CalculateDiscount(Amount)` — and let the old one delegate to the new one. Existing callers keep compiling; new callers opt into the richer entry point. Naming an unnamed return value is the one in-place change that is always safe.
@ -21,6 +23,6 @@ See sample: `do-not-change-published-procedure-signatures.good.al`.
## Anti Pattern
Editing the existing public procedure's parameter list — here, adding a `Rate` parameter to `CalculateDiscount` — so every dependent extension that called the old form fails to compile. Detection: a parameter added, removed, reordered, retyped, or flipped to/from `var`, or a changed return type, on any non-`local` procedure that already shipped. Add a new overload instead.
Editing the existing public procedure's parameter list — here, adding a `Rate` parameter to `CalculateDiscount` — so every dependent extension that called the old form fails to compile. Detection: a parameter added, removed, reordered, retyped, or flipped to/from `var`, or a changed return type, on any non-`local` procedure that already shipped. Add a new overload instead. Exclude event publishers whose only change is an added parameter: subscribers bind by parameter name, not position, so that edit is additive and reporting it here is a false positive.
See sample: `do-not-change-published-procedure-signatures.bad.al`.

24
microsoft/knowledge/data-modeling/insert-only-transfer-may-rely-on-caller-cleanup.md Normal file
View file

@ -0,0 +1,24 @@
---
bc-version: [all]
domain: data-modeling
keywords: [transfer, cleanup, deleteall, onvalidate, caller, stale-rows, false-positive]
technologies: [al]
countries: [w1]
application-area: [all]
---
# An insert-only transfer routine may rely on cleanup its caller already performed
## Description
A routine that copies rows from a template table into a target table — a `TransferX` procedure filling comment, dimension, or attribute lines from a standard-task or template record — is frequently written as filter-and-insert with no `DeleteAll` of its own. That is not automatically a stale-row or duplicate-primary-key defect. In the common AL shape, the field's `OnValidate` trigger first calls a sibling cleanup procedure that clears the same filtered range, then calls the transfer. By the time `Insert` runs, the target range is guaranteed empty, so the transfer has nothing to clean up and adding a second `DeleteAll` inside it would be redundant.
Deciding whether a missing cleanup is real therefore requires reading the caller, not just the routine in the diff. The relevant question is whether every path that reaches the transfer clears the target range first — not whether the transfer clears it itself.
## Best Practice
Before reporting a transfer or copy routine for missing cleanup, trace its call sites. If the callers in scope invoke a cleanup procedure that clears the same filtered range immediately beforehand — typically in the same `OnValidate` trigger or the same routine — the insert-only transfer is correct and must not be flagged for stale rows, duplicate keys, or a missing `DeleteAll`. Raise the finding only when a reachable call path inserts into a range that was not cleared, or when the cleanup filters a different range than the insert writes to.
## Anti Pattern
Reporting an insert-only transfer as a stale-row or duplicate-key risk on the strength of the routine body alone, when the trigger that calls it already ran the cleanup. The mirror-image mistake is waving through a transfer whose caller clears a *different* filter range than the one the transfer inserts into, or one reachable from a path with no cleanup at all — those are genuine defects.

0
community/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.bad.al → microsoft/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.bad.al
View file

0
community/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.good.al → microsoft/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.good.al
View file

0
community/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.md → microsoft/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.md
View file

0
community/knowledge/data-modeling/transferfields-skip-type-mismatch-can-drop-data.bad.al → microsoft/knowledge/data-modeling/transferfields-skip-type-mismatch-can-drop-data.bad.al
View file

0
community/knowledge/data-modeling/transferfields-skip-type-mismatch-can-drop-data.good.al → microsoft/knowledge/data-modeling/transferfields-skip-type-mismatch-can-drop-data.good.al
View file

0
community/knowledge/data-modeling/transferfields-skip-type-mismatch-can-drop-data.md → microsoft/knowledge/data-modeling/transferfields-skip-type-mismatch-can-drop-data.md
View file

0
community/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.bad.al → microsoft/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.bad.al
View file

0
community/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.good.al → microsoft/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.good.al
View file

0
community/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.md → microsoft/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.md
View file

0
community/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.bad.al → microsoft/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.bad.al
View file

0
community/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.good.al → microsoft/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.good.al
View file

0
community/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.md → microsoft/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.md
View file

4
microsoft/knowledge/error-handling/fielderror-vs-testfield.md
View file

@ -1,7 +1,7 @@
---
bc-version: [all]
domain: error-handling
keywords: [fielderror, testfield, field-validation, onvalidate, error-message, mandatory-field, record-context]
keywords: [fielderror, testfield, field-validation, onvalidate, error-message, mandatory-field, record-context, onaction, enabled-property]
technologies: [al]
countries: [w1]
application-area: [all]
@ -14,6 +14,8 @@ application-area: [all]
## Best Practice
Use `TestField` when the condition is a simple presence-or-equality check on a single field — mandatory-field gates and prerequisite checks at the top of a procedure read clearly and self-document intent. Use `FieldError` inside an `OnValidate` trigger or a validation procedure where surrounding business logic has already determined the value is invalid and you want a specific, custom message. Rely on the built-in field-and-record context both methods add rather than re-stating the field name in the text.
A page action's `OnAction` trigger is a different case: a page action is only invocable through its own UI control, so when the action's `Enabled` property is already bound to the same condition the trigger would otherwise `TestField`, the control cannot be clicked while the field is blank and the field can never reach the trigger empty. Adding a `TestField` there is redundant defensive code, not a missing check — flag it only when the trigger can run through a path `Enabled` does not cover (a shared procedure, an API, or a condition broader than what gates the action).
See sample: `fielderror-vs-testfield.good.al`.
## Anti Pattern

2
microsoft/knowledge/events/avoid-raising-events-inside-try-functions.bad.al
View file

@ -3,7 +3,7 @@
codeunit 50116 "Payment Processor Bad"
{
[IntegrationEvent(false, false)]
procedure OnBeforeSubmitPayment(var PaymentAmount: Decimal; var Cancel: Boolean)
local procedure OnBeforeSubmitPayment(var PaymentAmount: Decimal; var Cancel: Boolean)
begin
end;

2
microsoft/knowledge/events/avoid-raising-events-inside-try-functions.good.al
View file

@ -3,7 +3,7 @@
codeunit 50114 "Payment Processor"
{
[IntegrationEvent(false, false)]
procedure OnBeforeSubmitPayment(var PaymentAmount: Decimal; var Cancel: Boolean)
local procedure OnBeforeSubmitPayment(var PaymentAmount: Decimal; var Cancel: Boolean)
begin
end;

43
microsoft/knowledge/events/declare-event-publishers-local-or-internal.bad.al Normal file
View file

@ -0,0 +1,43 @@
// Demonstration only. Shows the wrong pattern: the publisher carries no access modifier, so it is
// public - which never was what lets extensions subscribe.
codeunit 50100 "Loyalty Points Mgt Bad"
{
procedure AwardPoints(CustomerNo: Code[20]; SalesAmount: Decimal)
var
Points: Decimal;
IsHandled: Boolean;
begin
Points := SalesAmount / 10;
IsHandled := false;
OnBeforeAwardPoints(CustomerNo, Points, IsHandled);
if IsHandled then
exit;
// ... insert the loyalty entry ...
end;
// BAD: no access modifier, so this publisher is public. Public access does not enable
// subscription - it enables raising. Narrowing it to internal after release breaks callers,
// so the widening cannot be walked back cheaply.
[IntegrationEvent(false, false)]
procedure OnBeforeAwardPoints(CustomerNo: Code[20]; var Points: Decimal; var IsHandled: Boolean)
begin
end;
}
codeunit 50101 "Loyalty Points Caller Bad"
{
procedure FirePublisherDirectly(CustomerNo: Code[20])
var
LoyaltyPointsMgt: Codeunit "Loyalty Points Mgt Bad";
Points: Decimal;
IsHandled: Boolean;
begin
// Compiles only because the publisher is public. Every subscriber runs although no points
// were ever awarded, on a Points value nobody computed, and the IsHandled answer the
// subscribers write is read by no one.
LoyaltyPointsMgt.OnBeforeAwardPoints(CustomerNo, Points, IsHandled);
end;
}

59
microsoft/knowledge/events/declare-event-publishers-local-or-internal.good.al Normal file
View file

@ -0,0 +1,59 @@
// Demonstration only. Shows the correct pattern: a public facade codeunit whose event publishers
// are internal, so only the implementation codeunit decides when they fire.
codeunit 50100 "Loyalty Points Mgt Good"
{
procedure AwardPoints(CustomerNo: Code[20]; SalesAmount: Decimal)
var
LoyaltyPointsImpl: Codeunit "Loyalty Points Impl Good";
begin
LoyaltyPointsImpl.AwardPoints(CustomerNo, SalesAmount);
end;
// internal, not public: the implementation codeunit raises this and nobody else. Subscribers
// bind through Codeunit::"Loyalty Points Mgt Good", which is public by default - that object
// access is all a subscriber in another extension needs.
[IntegrationEvent(false, false)]
internal procedure OnBeforeAwardPoints(CustomerNo: Code[20]; var Points: Decimal; var IsHandled: Boolean)
begin
end;
[IntegrationEvent(false, false)]
internal procedure OnAfterAwardPoints(CustomerNo: Code[20]; Points: Decimal)
begin
end;
}
codeunit 50101 "Loyalty Points Impl Good"
{
Access = Internal;
procedure AwardPoints(CustomerNo: Code[20]; SalesAmount: Decimal)
var
LoyaltyPointsMgt: Codeunit "Loyalty Points Mgt Good";
Points: Decimal;
IsHandled: Boolean;
begin
Points := SalesAmount / 10;
IsHandled := false;
LoyaltyPointsMgt.OnBeforeAwardPoints(CustomerNo, Points, IsHandled);
if IsHandled then
exit;
// ... insert the loyalty entry ...
LoyaltyPointsMgt.OnAfterAwardPoints(CustomerNo, Points);
end;
}
codeunit 50102 "Loyalty Points Sub Good"
{
// The shape a subscriber in a dependent extension takes: it names the public object, and is
// indifferent to the publisher being internal.
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Loyalty Points Mgt Good", 'OnAfterAwardPoints', '', false, false)]
local procedure LogAwardedPointsOnAfterAwardPoints(CustomerNo: Code[20]; Points: Decimal)
begin
// ... write telemetry ...
end;
}

42
microsoft/knowledge/events/declare-event-publishers-local-or-internal.md Normal file
View file

@ -0,0 +1,42 @@
---
bc-version: [all]
domain: events
keywords: [event-publisher, access-modifier, local, internal, integration-event, business-event, subscriber, breaking-change]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Declare event publishers local or internal
> Contributions welcome — open a PR to refine or extend this article.
## Description
The access modifier on an `[IntegrationEvent]` or `[BusinessEvent]` publisher controls who may *raise* the procedure, not who may *subscribe* to it. A subscriber in a dependent extension binds through the object named in its `[EventSubscriber(...)]` attribute, so the only accessibility a foreign subscriber needs is on the *object* — a codeunit left at its default public access. The publisher procedure itself can and should stay `local` or `internal`. Publishing an event is an invitation to subscribe, not an invitation to call: an omitted access modifier makes the publisher public, which hands every dependent extension the ability to fire the event on its own. The signature-compatibility consequences of a shipped publisher are covered separately by `treat-local-and-internal-events-as-subscriber-contracts`.
## Applies to
Ordinary `[IntegrationEvent]` and `[BusinessEvent]` publishers. `[InternalEvent]` has its own module-only visibility semantics, and external business events are out of scope.
## Best Practice
Give an event publisher the narrowest access modifier that still lets the code owning the operation raise it:
- `local` when only the declaring object raises the event. This is the common case and the default choice.
- `internal` when another object in the same app raises it — typically an internal implementation codeunit raising an event declared on a public facade codeunit. The facade object stays public so dependent extensions can name it in `[EventSubscriber(...)]`; the publisher stays `internal` so only the implementation decides when the event fires.
- `public` only when a *different app* must raise the event — a hub or event-bus codeunit in a foundation app that sibling apps signal through, where `internal` cannot reach across the app boundary. This is a deliberate caller contract, not a concession to subscribers, and it is maintained like any other public API.
Subscribers are unaffected by any of these choices. A non-public publisher also keeps the freedom to add a parameter later, which a public publisher gives up — see `add-new-event-parameters-at-the-end`.
See sample: `declare-event-publishers-local-or-internal.good.al`.
## Anti Pattern
An event publisher declared with no access modifier — or widened to public — in the belief that dependent extensions need that to subscribe. They do not. Two consequences follow. Any dependent extension can now call the publisher directly, firing every subscriber outside the owning routine's control flow, on state the publisher never prepared and with an `IsHandled` answer nobody reads. And because the publisher is a public procedure, it is a caller contract: narrowing it back to `local` or `internal` after release is itself a breaking change, so the mistake is not cheaply reversible.
Detection: an `[IntegrationEvent]` or `[BusinessEvent]` publisher that is public although every raiser is in its own app — typically raised only from its declaring object. A publisher deliberately made public so another app can raise it is not this anti-pattern; do not report it. When the surrounding repository or API context does not reveal whether an external raiser is intended, treat the public modifier as intentional rather than reporting it.
The mirror-image anti-pattern belongs to the reviewer, human or agent: recommending that a publisher be made public so extensions can subscribe, or reporting a `local`/`internal` publisher as unreachable dead code. Both readings mistake raising for subscribing. Neither should be raised as a finding.
See sample: `declare-event-publishers-local-or-internal.bad.al`.

72
microsoft/knowledge/events/expose-process-context-via-manually-bound-flag.bad.al Normal file
View file

@ -0,0 +1,72 @@
// Demonstration-only AL. Not compiled by CI; illustrates the article.
// Anti-pattern 1: the context is kept in single-instance state.
codeunit 50545 "Process State Bad Sample"
{
SingleInstance = true;
var
ProcessRunning: Boolean;
procedure SetProcessRunning(NewProcessRunning: Boolean)
begin
ProcessRunning := NewProcessRunning;
end;
procedure IsProcessRunning(): Boolean
begin
exit(ProcessRunning);
end;
}
codeunit 50546 "Process Driver Bad Sample"
{
procedure Run(DocumentNo: Code[20])
var
ProcessState: Codeunit "Process State Bad Sample";
begin
ProcessState.SetProcessRunning(true);
RunSharedCode(DocumentNo);
// An error above never reaches this line. The database writes roll
// back, the single-instance variable does not: ProcessRunning stays
// true until the company is closed, so every later run in this session
// is treated as part of the process.
ProcessState.SetProcessRunning(false);
end;
local procedure RunSharedCode(DocumentNo: Code[20])
begin
end;
}
// Anti-pattern 2: the context stays private. Flag and driver look like the
// good sample, but the query is internal, so only the owning app can ever ask.
codeunit 50547 "Process Ctx Bad Sample"
{
internal procedure IsProcessRunning(): Boolean
var
IsRunning: Boolean;
begin
OnCheckProcessRunning(IsRunning);
exit(IsRunning);
end;
[InternalEvent(false)]
local procedure OnCheckProcessRunning(var IsRunning: Boolean)
begin
end;
}
reportextension 50548 "Shared Report Ext Bad Sample" extends "Standard Sales - Invoice"
{
trigger OnPreReport()
begin
// No callable query exists, so the extension infers the context from
// something it hopes only that process does - here, running without a
// UI. The guess is wrong for every other background run, and breaks
// silently the first time the owning app changes how it works.
if GuiAllowed() then
exit;
// ... behaviour that was meant to apply only inside that process ...
end;
}

70
microsoft/knowledge/events/expose-process-context-via-manually-bound-flag.good.al Normal file
View file

@ -0,0 +1,70 @@
// Demonstration-only AL. Not compiled by CI; illustrates the article.
// The published context API - the entire public surface of the pattern.
// Any dependent extension may call IsProcessRunning; nothing else is exposed.
codeunit 50540 "Process Context Good Sample"
{
procedure IsProcessRunning(): Boolean
var
IsRunning: Boolean;
begin
OnCheckProcessRunning(IsRunning);
exit(IsRunning);
end;
// InternalEvent: only this app can subscribe, which is all the pattern
// needs. local: only this codeunit can raise it.
[InternalEvent(false)]
local procedure OnCheckProcessRunning(var IsRunning: Boolean)
begin
end;
}
// The flag - implementation, not API, hence Access = Internal. It stores
// nothing between runs: being bound is the state.
codeunit 50541 "Process Flag Good Sample"
{
Access = Internal;
EventSubscriberInstance = Manual;
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Process Context Good Sample", 'OnCheckProcessRunning', '', false, false)]
local procedure SetProcessRunning(var IsRunning: Boolean)
begin
IsRunning := true;
end;
}
// The app that drives the process claims the context for exactly its own run.
codeunit 50542 "Process Driver Good Sample"
{
procedure Run(DocumentNo: Code[20])
var
ProcessFlag: Codeunit "Process Flag Good Sample";
begin
// A fresh instance, bound for exactly this call. If the shared code
// errors, the stack unwinds and takes the binding with it - nothing to reset.
BindSubscription(ProcessFlag);
RunSharedCode(DocumentNo);
end;
local procedure RunSharedCode(DocumentNo: Code[20])
begin
// A base application report, a posting routine, or any other object
// that extensions hook into - including a customer's own replacement.
end;
}
// An extension hooked into that shared code can now ask the question directly
// instead of guessing which process is driving the run. The hook happens to be
// a report extension here; a subscriber on any other shared object is the same.
reportextension 50543 "Shared Report Ext Good Sample" extends "Standard Sales - Invoice"
{
trigger OnPreReport()
var
ProcessContext: Codeunit "Process Context Good Sample";
begin
if not ProcessContext.IsProcessRunning() then
exit;
// ... behaviour that applies only inside that process ...
end;
}

40
microsoft/knowledge/events/expose-process-context-via-manually-bound-flag.md Normal file
View file

@ -0,0 +1,40 @@
---
bc-version: [all]
domain: events
keywords: [bindsubscription, manual-binding, eventsubscriberinstance, internalevent, singleinstance, process-context, running-flag, scoped-state, rollback]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Expose process context through a manually bound flag, not a single-instance boolean
> Contributions welcome — open a PR to refine or extend this article.
## Description
When an extension drives a process over shared code — a base application report, a posting routine — other extensions hooked into that code cannot tell whether a run belongs to that process: AL keeps no ambient "current process", so the driving app has to publish the context itself. The reflex answer, a `SingleInstance` codeunit holding a boolean set at the start of the run and cleared at the end, is unsafe: single-instance variables are not part of the database transaction, so a failed run rolls back the writes but not the flag, which stays `true` until the company is closed and marks every later run in the session as part of the process. A manual event binding carries the same signal safely, because the platform ties its lifetime to a variable's scope instead of to cleanup code that has to run.
## Best Practice
Publish the context as a query and let the binding itself be the state. One procedure is public; everything behind it is internal:
- A public context codeunit exposes `IsProcessRunning(): Boolean`, which raises an `[InternalEvent]` publisher taking a `var Boolean` and returns what comes back — the entire public surface. The publisher is internal because only the owning app subscribes, `local` because only this codeunit raises it.
- A second codeunit, `Access = Internal` with `EventSubscriberInstance = Manual`, subscribes to that event and sets the boolean to `true`. Internal keeps it out of the API and stops other apps binding it to forge the context; it stores nothing between runs — being bound *is* the state.
- The driving process calls `BindSubscription` on a variable whose scope is exactly the span it wants to claim: a local in the procedure that drives the run, or a global on an object that lives exactly as long as the run. While that variable is alive the query answers `true`; when it leaves scope — normally, or because an error unwound the call stack — the platform removes the binding and the query answers `false` again.
Bind a fresh instance per run rather than reusing one: the platform refuses to bind the same instance twice but accepts several instances of the same codeunit, so nesting and re-entrancy need no counter. The binding is session-scoped, so work the process starts in another session — a background session, a page background task, a job queue entry — cannot see it; pass the context explicitly there.
See sample: `expose-process-context-via-manually-bound-flag.good.al`.
## Anti Pattern
Two shapes.
First, the single-instance boolean — the failure described above. Detection: a `SingleInstance = true` codeunit with a boolean set before a process and cleared after it, read by other code to decide whether that process is running.
Second, the context kept private: the driving app arranges its own marker — typically a manually bound subscriber on an event added for its benefit alone — and offers no query, or only an `internal` one. Other extensions are left inferring the context from side effects, request-page values, or record state, which breaks silently the first time the process changes. Detection: a manual binding used purely as an internal run marker, with no public query procedure over it.
The mirror-image anti-pattern belongs to the reviewer: flagging the `BindSubscription` here as a leaked binding because no `UnbindSubscription` follows it. Scope release is the mechanism, not an omission — see `microsoft/knowledge/events/choose-static-vs-manual-subscribers-deliberately.md`, whose leak case is an instance parked on a `SingleInstance` global that never leaves scope.
See sample: `expose-process-context-via-manually-bound-flag.bad.al`.

31
microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.bad.al
View file

@ -1,31 +0,0 @@
// Demonstration-only AL. Not compiled by CI; illustrates the article.
codeunit 50241 "IsHandled Init Bad Sample"
{
procedure ApplyDiscounts(var SalesHeader: Record "Sales Header")
var
DiscountPct: Decimal;
IsHandled: Boolean;
begin
// IsHandled is never initialized before the first raise, so flow depends
// on the variable's default rather than an explicit, documented intent.
OnBeforeApplyHeaderDiscount(SalesHeader, DiscountPct, IsHandled);
if not IsHandled then
DiscountPct := 5;
// Bug: IsHandled is not reset. If the first subscriber set it true, the
// payment-discount default below is silently skipped too.
OnBeforeApplyPaymentDiscount(SalesHeader, DiscountPct, IsHandled);
if not IsHandled then
DiscountPct += 2;
end;
[IntegrationEvent(false, false)]
local procedure OnBeforeApplyHeaderDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
begin
end;
[IntegrationEvent(false, false)]
local procedure OnBeforeApplyPaymentDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
begin
end;
}

31
microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.good.al
View file

@ -1,31 +0,0 @@
// Demonstration-only AL. Not compiled by CI; illustrates the article.
codeunit 50240 "IsHandled Init Good Sample"
{
procedure ApplyDiscounts(var SalesHeader: Record "Sales Header")
var
DiscountPct: Decimal;
IsHandled: Boolean;
begin
IsHandled := false;
OnBeforeApplyHeaderDiscount(SalesHeader, DiscountPct, IsHandled);
if not IsHandled then
DiscountPct := 5;
// Reset before reusing the same variable for the next event so a
// subscriber that handled the first raise can't suppress this one.
IsHandled := false;
OnBeforeApplyPaymentDiscount(SalesHeader, DiscountPct, IsHandled);
if not IsHandled then
DiscountPct += 2;
end;
[IntegrationEvent(false, false)]
local procedure OnBeforeApplyHeaderDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
begin
end;
[IntegrationEvent(false, false)]
local procedure OnBeforeApplyPaymentDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
begin
end;
}

26
microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: events
keywords: [ishandled, initialization, deterministic, onbefore, reset, integration-event, control-flow]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Initialize IsHandled to false before publishing
## Description
A routine that raises an `OnBefore…` integration event with a `var IsHandled: Boolean` parameter passes that variable in by reference, so its incoming value decides whether the default logic is skipped. A freshly declared Boolean starts as `false`, but the same variable is frequently reused to raise several events in one routine, and after the first raise it may already be `true`. Assigning `IsHandled := false;` on the line immediately before every raise makes the control flow deterministic and self-documenting, and prevents a stale `true` from silently suppressing logic the author never meant to make skippable. Generated code often reuses one `IsHandled` across several raises without resetting it.
## Best Practice
Set `IsHandled := false;` immediately before each `OnBeforeX(…, IsHandled)` raise, then guard the default logic with `if IsHandled then exit;` or `if not IsHandled then …`. Do this even when the variable was just declared: the explicit reset documents intent and stays correct if a second event raise is added to the routine later. This applies only to events that carry a `var IsHandled: Boolean`; an `OnBefore` event with no `IsHandled` parameter needs no reset.
See sample: `initialize-ishandled-to-false-before-publishing.good.al`.
## Anti Pattern
Raising `OnBeforeX(…, IsHandled)` with a variable whose value carries over from an earlier raise, so a subscriber that handled the first event unintentionally suppresses the second routine's default logic. Detection: an `IsHandled` variable passed to more than one event in a routine without an intervening `IsHandled := false;`, or any `OnBefore…` raise that passes an `IsHandled` variable without an intervening `IsHandled := false;`.
See sample: `initialize-ishandled-to-false-before-publishing.bad.al`.

48
microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.bad.al Normal file
View file

@ -0,0 +1,48 @@
// Demonstration-only AL. Not compiled by CI; illustrates the article.
codeunit 50241 "IsHandled Carry Over Bad Sample"
{
procedure ApplyDiscounts(var SalesHeader: Record "Sales Header")
var
DiscountPct: Decimal;
IsHandled: Boolean;
begin
OnBeforeApplyHeaderDiscount(SalesHeader, DiscountPct, IsHandled);
if not IsHandled then
DiscountPct := 5;
// Bug: execution continues when the first event set IsHandled to true,
// and that stale value is passed to a different publisher.
OnBeforeApplyPaymentDiscount(SalesHeader, DiscountPct, IsHandled);
if not IsHandled then
DiscountPct += 2;
end;
procedure ApplyLineDiscounts(var SalesLine: Record "Sales Line")
var
LineIsHandled: Boolean;
begin
if SalesLine.FindSet() then
repeat
// Bug: the local initializes only once. A subscriber that handles
// one line leaves true for every later iteration.
OnBeforeApplyLineDiscount(SalesLine, LineIsHandled);
if not LineIsHandled then
SalesLine.Validate("Line Discount %", 5);
until SalesLine.Next() = 0;
end;
[IntegrationEvent(false, false)]
local procedure OnBeforeApplyHeaderDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
begin
end;
[IntegrationEvent(false, false)]
local procedure OnBeforeApplyPaymentDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
begin
end;
[IntegrationEvent(false, false)]
local procedure OnBeforeApplyLineDiscount(var SalesLine: Record "Sales Line"; var IsHandled: Boolean)
begin
end;
}

50
microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.good.al Normal file
View file

@ -0,0 +1,50 @@
// Demonstration-only AL. Not compiled by CI; illustrates the article.
codeunit 50240 "IsHandled Carry Over Good Sample"
{
procedure ApplyDiscounts(var SalesHeader: Record "Sales Header")
var
DiscountPct: Decimal;
HeaderIsHandled: Boolean;
PaymentIsHandled: Boolean;
begin
// Each fresh local is false and belongs to one non-looping raise.
OnBeforeApplyHeaderDiscount(SalesHeader, DiscountPct, HeaderIsHandled);
if not HeaderIsHandled then
DiscountPct := 5;
// Handling the header event does not suppress this independent seam.
OnBeforeApplyPaymentDiscount(SalesHeader, DiscountPct, PaymentIsHandled);
if not PaymentIsHandled then
DiscountPct += 2;
end;
procedure ApplyLineDiscounts(var SalesLine: Record "Sales Line")
var
LineIsHandled: Boolean;
begin
if SalesLine.FindSet() then
repeat
// The local initializes once, so reset it per iteration; a
// subscriber that handles one line must not skip the rest.
LineIsHandled := false;
OnBeforeApplyLineDiscount(SalesLine, LineIsHandled);
if not LineIsHandled then
SalesLine.Validate("Line Discount %", 5);
until SalesLine.Next() = 0;
end;
[IntegrationEvent(false, false)]
local procedure OnBeforeApplyHeaderDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
begin
end;
[IntegrationEvent(false, false)]
local procedure OnBeforeApplyPaymentDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
begin
end;
[IntegrationEvent(false, false)]
local procedure OnBeforeApplyLineDiscount(var SalesLine: Record "Sales Line"; var IsHandled: Boolean)
begin
end;
}

26
microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.md Normal file
View file

@ -0,0 +1,26 @@
---
bc-version: [all]
domain: events
keywords: [ishandled, carry-over, loop-iteration, onbefore, reset, integration-event, control-flow, false-positive]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Reset IsHandled before publishing only when its value can carry over
## Description
A routine that raises an `OnBefore…` integration event with a `var IsHandled: Boolean` parameter passes that variable by reference, so a pre-existing `true` can affect the following control flow. AL [automatically initializes Boolean variables to `false`](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-al-variables#initialization), so a freshly declared local Boolean passed to one event exactly once per procedure invocation is already deterministic. Initialization does not repeat for each loop iteration: a local declared outside a loop can carry `true` from one iteration to the next even when the source contains only one textual event raise. Outside a loop, reaching a later raise after `if IsHandled then exit;` also proves the value is `false`, provided that early exit is semantically correct and does not skip required downstream events.
## Best Practice
Reset `IsHandled := false;` before a raise only when the value might otherwise carry over as `true`: the same variable is reused after an earlier raise without a control-flow proof that it is false, a raise is re-entered by a loop, the value comes from an input parameter, field, or global, or earlier code seeds it. Prefer separate fresh locals when independent event seams need independent handled state. A reset on a guaranteed-false fresh local used by one non-looping raise, or before a later raise reached only after a semantically valid `if IsHandled then exit;`, can be retained for readability, but its absence is not a correctness finding.
See sample: `reset-ishandled-only-when-the-value-can-carry-over.good.al`.
## Anti Pattern
Raising `OnBeforeX(…, IsHandled)` when the variable can still be `true` from an earlier raise, an earlier loop iteration, or another source, so the publisher call starts with stale state. Do not match a single non-looping raise using a fresh local Boolean, or a later raise reached only after a semantically valid `if IsHandled then exit;` proves the value is false.
See sample: `reset-ishandled-only-when-the-value-can-carry-over.bad.al`.

2
microsoft/knowledge/performance/avoid-cloning-records-before-modify-delete-in-loops.bad.al
View file

@ -5,8 +5,8 @@ codeunit 50493 "Perf Record Clone Bad"
Customer: Record Customer;
CustomerCopy: Record Customer;
begin
Customer.SetLoadFields("Credit Limit (LCY)");
Customer.SetFilter("Credit Limit (LCY)", '>0');
Customer.SetLoadFields("Credit Limit (LCY)");
if Customer.FindSet(true) then
repeat
CustomerCopy.Copy(Customer);

2
microsoft/knowledge/performance/avoid-cloning-records-before-modify-delete-in-loops.good.al
View file

@ -4,8 +4,8 @@ codeunit 50492 "Perf Record Clone Good"
var
Customer: Record Customer;
begin
Customer.SetLoadFields("Credit Limit (LCY)");
Customer.SetFilter("Credit Limit (LCY)", '>0');
Customer.SetLoadFields("Credit Limit (LCY)");
if Customer.FindSet(true) then
repeat
Customer.Validate(

Some files were not shown because too many files have changed in this diff Show more