Add community guidance and review support for Business Central agents

[Stefano Demiliani (demiliani)]

Summary

Adds a new community knowledge domain for reviewing Business Central Agent SDK implementations.

The contribution includes 20 guidance rules, each with documentation and good/bad AL examples, covering:

• Agent registration and interface wiring

• Profiles and setup-page design

• Setup persistence and user identity

• Least-privilege permissions and effective access

• Instruction structure, keywords, and secure storage

• Agent lifecycle and upgrade behavior

• Session-bound subscribers and input review

• Cross-app APIs and Copilot capability registration

• Programmatic agent creation safeguards

• Error and warning handling

• Adds the standalone community-owned al-agents-review action skill, dispatched by Entry alongside al-code-review.

• Keeps the Agents domain under the community layer.

[Stefano Demiliani (demiliani)]

Anyone can check what gate is blocking this PR? Thanks in advance.

[Jesper Schulz-Wedde (JesperSchulz)]

Anyone can check what gate is blocking this PR? Thanks in advance.

All green. We just need some reviews ☺️ Let me get on that, and ask others to join the fun!

[Jesper Schulz-Wedde (JesperSchulz)]

Thorough pass over this one — read all 20 articles and 40 samples, ran every CI validator locally on Windows, and verified the Agent SDK claims against MS Learn, microsoft/ALAppExtensions (f86dcaf) and microsoft/BCTech (7b1360e).

The knowledge itself is good. It's genuinely remedial and BC-specific, there's zero overlap with microsoft/knowledge, every article carries a Detection signal: line, and the ## See also cross-links are used well. 16 of the 17 factual claims I checked came back verified — including the subtle one I expected to be wrong: Agent.Create takes Record "Agent Access Control" while GetDefaultAccessControls takes Record "Access Control Buffer". Those really are two different records for two different concerns, and all six samples get it right.

Requesting changes on two blocking items plus a few sample-correctness fixes. Details inline; three things that have no line to attach to are below.

---

The PR description overstates what's wired up

"Enables Agents findings in AL code reviews" implies this leaf runs inside al-code-review. It doesn't — al-code-review.md:42 says sub-skills are never discovered implicitly, and the microsoft layer can't reference community, so it never will.

What actually happens is that Entry dispatches it as a top-level peer of al-code-review. That works, but it's a different mechanism with different properties (see the comment on al-agents-review.md). Could you reword to something like "Adds a standalone al-agents-review action skill, dispatched by Entry alongside al-code-review"?

Not a blocker for this PR, but worth noting: there's currently no sanctioned way for any community review leaf to join a microsoft super-skill's rollup. Cross-layer composition only works downward, and custom lives in consumer repos. If community review leaves are meant to be a supported pattern, al-code-review probably needs an extension point — sourcing sub-skills by domain across enabled layers rather than by hardcoded path. Happy to open a separate issue.

The auto-selected evaluation fixture is the weakest pair in the set

evaluation/review-fixtures.json has no overrides entry for agents, so the harness takes the first article alphabetically: agent-permissions-intersect-with-assigner. Its .bad.al is a bare 12-line permissionset with no agent context at all — more naturally an al-security-review finding — and the .good.al is a 7-line permissionset.

With minimumExpectedRecall: 1.0 and minimumCleanRate: 1.0 that's a fragile control. 10 of the 16 existing domains already carry an override for exactly this reason. wire-all-three-agent-interfaces or do-not-create-agents-in-install-upgrade-or-background would both be much sharper.

Smaller things

• README.md:93 — the Scope paragraph enumerates the covered domains and wasn't updated for agents.
• bc-version — 18 articles are [27..], but the SDK is sandbox-only from 27.4 and production-supported from 28.1, so [27..] over-claims 27.0–27.3. The schema only supports majors, so this may just be unavoidable; flagging it in case you want the two [28..] articles to set the floor for everything.
• al-agents-review.md doesn't define a not-applicable gate even though it lists that outcome. al-ui-review.md has an explicit file-type gate; without one, this fairly niche leaf evaluates every AL diff. (The missing json Output example is fine — al-query-review.md has none either.)

CI note

flag and guard show as failing in the checks list, but both are actually cancelled — they're pull_request_target workflows from a fork waiting on maintainer approval, not content failures. validate, validate-index and validate-review-fixtures all genuinely pass.

[Jesper Schulz-Wedde (JesperSchulz)]

Re-reviewed at e98fb8e. Everything from the previous pass is addressed — verified each fix against the diff, and re-ran the full validator suite on Windows including the -PrepareDirectory path that was broken before.

Blocking items — both resolved

• Test-ReviewFixtures.ps1:230 now normalizes the separator. I re-ran the CI mode on Windows and it passes clean: Prepared 34 neutral fixture(s) / PASSED: 34 cases cover 17 leaf domains.
• al-agents-review.md now states the dispatch relationship outright ("It is not one of the skills composed by al-code-review; Entry discovers and dispatches it as a top-level peer"), and the PR description matches. Thanks for taking the wording.

Sample correctness — all three fixed correctly

• The bind pair is now coherent: the good sample defines its own codeunit 50101 "Sales Review Agent Subscribers" with EventSubscriberInstance = Manual, and the bad sample is left as a plain static subscriber. The property is now the visible difference between the two, which is exactly what the article is teaching.
• do-not-create-agents…good.al goes through GetAgentSetupBuffer → AgentSetup.SaveChanges, matching BCTech SalesValAgentSetup.Page.al. It no longer contradicts agent-setup-source-table-is-temporary.good.al, and the unpopulated access-control record is gone.
• analyze-message…good.al uses #13#10, demonstrates both the Error and Warning branches, and adds Clear(Annotations) before each populate. IsRelevant doing a real substring test instead of <> '' is a nice extra — the clean control is more convincing for it.

The fixture override is a big improvement

Swapping in wire-all-three-agent-interfaces gives the harness a genuinely sharp pair. The generated positive control is an enumextension wiring only IAgentFactory, against a clean control wiring all three — unambiguous in both directions, which is what you want at minimumExpectedRecall: 1.0 / minimumCleanRate: 1.0.

Also nice: the not-applicable gate I mentioned as a nit got a real definition rather than a hand-wave, and the override error branch in Test-ReviewFixtures.ps1 now distinguishes "doesn't exist" from "exists but has no companions" and continues, so it no longer double-reports.

All six checks are green, and flag/guard now run for real rather than sitting cancelled.

One cosmetic leftover, definitely not worth another round: register-copilot-capability-for-the-agent.bad.al:7 still has IAgentMetadata = "Sales Review Agent Metadata" from before the rename. Harmless in a bad sample — fold it in if you touch the file for something else.

Nice contribution. LGTM.