mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 06:36:55 +01:00
Compare commits
45 commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ac249ba4c9 | ||
|
|
488ce50775 | ||
|
|
206c5feff4 | ||
|
|
0867171b1a | ||
|
|
a2685b9c86 | ||
|
|
fd59919778 | ||
|
|
164b27d0b2 | ||
|
|
87ba36e650 | ||
|
|
56ce52a9c7 | ||
|
|
830eaff68f | ||
|
|
f63943dcfd | ||
|
|
4287233f80 | ||
|
|
130d5de6c4 | ||
|
|
186691f815 | ||
|
|
0a8c9a8556 | ||
|
|
07e324ddbc | ||
|
|
bec8890b7e | ||
|
|
d38377b85e | ||
|
|
dd833133e0 | ||
|
|
38ad6b8810 | ||
|
|
523fc88f87 | ||
|
|
b91443beec | ||
|
|
d209cd0f73 | ||
|
|
2b96f5226d | ||
|
|
450e5965e1 | ||
|
|
8ff2326b61 | ||
|
|
5bda054927 | ||
|
|
58b3be23ab | ||
|
|
2c45021cb3 | ||
|
|
861f53dd97 | ||
|
|
b7617fb48a | ||
|
|
d24dc7b14b | ||
|
|
b545b22fb9 | ||
|
|
b74967bc5b | ||
|
|
852a676285 | ||
|
|
8c26ba4e76 | ||
|
|
45ac371e7a | ||
|
|
35d0966a8d | ||
|
|
51597068b1 | ||
|
|
c12b2f0a88 | ||
|
|
ac9e4fd9a2 | ||
|
|
2b5550c346 | ||
|
|
a21edfec46 | ||
|
|
17bb84a25e | ||
|
|
8584217c75 |
716 changed files with 19038 additions and 2008 deletions
36
.github/custom-layer-autoclose.md
vendored
36
.github/custom-layer-autoclose.md
vendored
|
|
@ -1,23 +1,19 @@
|
||||||
Hey @{{AUTHOR}} 👋
|
Thank you for contributing, @{{AUTHOR}}.
|
||||||
|
|
||||||
First off — thank you for jumping in and experimenting! It's awesome to see people pushing on the framework. 🎉
|
This PR was closed automatically because it changes custom-layer content.
|
||||||
|
`custom/` is reserved for organization-specific knowledge and skills in
|
||||||
|
**your own fork**, not the shared upstream repository.
|
||||||
|
|
||||||
That said, let me gently redirect you, because I think there's a small but important misunderstanding about how the `custom` layer is meant to work:
|
Keep company-only rules in your fork and point your host at that copy. The
|
||||||
|
[customization guide](https://github.com/microsoft/BCQuality/blob/main/docs/customizing-bcquality.md)
|
||||||
|
shows the complete flow.
|
||||||
|
|
||||||
The `custom` layer in *this* repo isn't a destination for PRs — it's the designated sandbox inside **your own fork**. Think of it as the "your timeline" branch of the multiverse 🌌: this repo is canon, your fork is where you get to remix the lore without needing anyone's approval. That's the whole point of the layer existing — so you *don't* have to upstream your team-specific or experimental work.
|
If the guidance is useful to everyone, submit it to the layer that owns the
|
||||||
|
domain: Microsoft-owned domains belong under `microsoft/knowledge/`, even
|
||||||
The intended workflow is:
|
when contributed by a partner; Community-owned domains belong under
|
||||||
|
`community/knowledge/`. See
|
||||||
1. 🍴 **Fork** BCQuality to your own GitHub account
|
[Contributing](https://github.com/microsoft/BCQuality/blob/main/docs/contributing.md).
|
||||||
2. Clone *your fork* locally
|
BCQuality contains knowledge and skills, not agents.
|
||||||
3. Drop your custom agents and knowledge into the `custom` layer **there**
|
|
||||||
4. Commit and push to your fork — no PR back to upstream needed for custom stuff
|
|
||||||
|
|
||||||
That way you get full control, your changes survive upstream updates cleanly, and you can pull in new core releases from this repo whenever you want. ✨
|
|
||||||
|
|
||||||
**Now — here's the fun part:** if while building out your fork you discover knowledge, patterns, or agents that you think would genuinely benefit *everyone* using BCQuality (not just your team), that's exactly what the `/community` layer is for! 🌟 PRs to `/community` here in the upstream repo are absolutely welcome and encouraged — it's how the collective hive mind 🧠 levels up. So please: tinker in your fork, and when you strike gold that's worth sharing, send it our way via `/community`.
|
|
||||||
|
|
||||||
Going to close this PR for now (since it's targeting `custom` rather than `/community`), but please don't read it as a "no" — it's a "yes, but let's route it correctly." 🙏 Happy to help if you hit any snags spinning up your fork, and genuinely looking forward to seeing what you contribute to `/community` down the line.
|
|
||||||
|
|
||||||
<details>
|
<details>
|
||||||
<summary>Files in this PR that triggered the auto-close</summary>
|
<summary>Files in this PR that triggered the auto-close</summary>
|
||||||
|
|
@ -25,7 +21,5 @@ Going to close this PR for now (since it's targeting `custom` rather than `/comm
|
||||||
{{FILES}}
|
{{FILES}}
|
||||||
</details>
|
</details>
|
||||||
|
|
||||||
May your merges be conflict-free. 🚀
|
Template changes to `custom/README.md` and `.gitkeep` files are allowed. If
|
||||||
|
you believe this closure was a mistake, comment here for maintainer review.
|
||||||
---
|
|
||||||
<sub>🤖 This PR was closed automatically by the `Guard custom layer` workflow because it adds or changes content under `/custom/`. If you were only updating the template (`custom/README.md` or a `.gitkeep`), a maintainer can re-open it. If you think this was closed in error, just comment here.</sub>
|
|
||||||
|
|
|
||||||
2
.github/new-top-level-flag.md
vendored
2
.github/new-top-level-flag.md
vendored
|
|
@ -3,7 +3,7 @@
|
||||||
|
|
||||||
{{ENTRIES}}
|
{{ENTRIES}}
|
||||||
|
|
||||||
This isn't a block — just a flag. 🚩 New top-level folders and files are *usually* unintended (a stray export, a tool's scratch dir, or content that meant to land inside an existing layer like `/community/knowledge/`). BCQuality keeps a deliberately small root: `.github/`, `community/`, `custom/`, `microsoft/`, `skills/`, and `tools/`, plus a handful of root docs.
|
This isn't a block — just a flag. 🚩 New top-level folders and files are *usually* unintended (a stray export, a tool's scratch dir, or content that meant to land inside an existing layer). BCQuality keeps a deliberately small root: plugin metadata, `community/`, `custom/`, `microsoft/`, `skills/`, `tools/`, `docs/`, `evaluation/`, `.github/`, and a handful of root docs. Partner guides belong under `docs/`; shared knowledge belongs beside the skill that owns its domain.
|
||||||
|
|
||||||
**If this was intentional** and the new entry genuinely belongs at the repo root, a maintainer can review and merge as normal — no action needed beyond a quick sanity check. **If it wasn't**, please move the content into the right existing layer (or drop it) and push an update. 🙏
|
**If this was intentional** and the new entry genuinely belongs at the repo root, a maintainer can review and merge as normal — no action needed beyond a quick sanity check. **If it wasn't**, please move the content into the right existing layer (or drop it) and push an update. 🙏
|
||||||
|
|
||||||
|
|
|
||||||
3
.github/scripts/Test-KnowledgeIndex.ps1
vendored
3
.github/scripts/Test-KnowledgeIndex.ps1
vendored
|
|
@ -16,6 +16,8 @@
|
||||||
3. Selection-input integrity — every parsed article row carries the
|
3. Selection-input integrity — every parsed article row carries the
|
||||||
non-empty `domain` + `keywords` the worklist predicate selects on, and
|
non-empty `domain` + `keywords` the worklist predicate selects on, and
|
||||||
every article parses (an unparseable article is an invalid file).
|
every article parses (an unparseable article is an invalid file).
|
||||||
|
4. Bounded retrieval — delegates to tools/Test-KnowledgeRetrieval.ps1 for
|
||||||
|
lossless paging, exact-body round trips, and explicit failure cases.
|
||||||
|
|
||||||
Exit code 0 = healthy; non-zero = a problem CI must block on.
|
Exit code 0 = healthy; non-zero = a problem CI must block on.
|
||||||
#>
|
#>
|
||||||
|
|
@ -90,4 +92,5 @@ if ($problems.Count) {
|
||||||
exit 1
|
exit 1
|
||||||
}
|
}
|
||||||
Write-Host "Knowledge-index check PASSED: $($rows.Count) articles, deterministic, full coverage, selection inputs intact." -ForegroundColor Green
|
Write-Host "Knowledge-index check PASSED: $($rows.Count) articles, deterministic, full coverage, selection inputs intact." -ForegroundColor Green
|
||||||
|
& (Join-Path $Root 'tools/Test-KnowledgeRetrieval.ps1') -Root $Root
|
||||||
exit 0
|
exit 0
|
||||||
|
|
|
||||||
318
.github/scripts/Test-SkillIndex.ps1
vendored
Normal file
318
.github/scripts/Test-SkillIndex.ps1
vendored
Normal file
|
|
@ -0,0 +1,318 @@
|
||||||
|
<#
|
||||||
|
.SYNOPSIS
|
||||||
|
Validates the BCQuality action-skill index generator and shared schemas.
|
||||||
|
#>
|
||||||
|
[CmdletBinding()]
|
||||||
|
param(
|
||||||
|
[string] $Root = (Resolve-Path (Join-Path -Path $PSScriptRoot -ChildPath '..' '..'))
|
||||||
|
)
|
||||||
|
|
||||||
|
Set-StrictMode -Version Latest
|
||||||
|
$ErrorActionPreference = 'Stop'
|
||||||
|
$Root = (Resolve-Path -LiteralPath $Root).Path
|
||||||
|
|
||||||
|
function Assert-ThrowsLike {
|
||||||
|
param(
|
||||||
|
[scriptblock] $Action,
|
||||||
|
[string] $Pattern
|
||||||
|
)
|
||||||
|
|
||||||
|
try {
|
||||||
|
& $Action
|
||||||
|
}
|
||||||
|
catch {
|
||||||
|
if ($_.Exception.Message -like $Pattern) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
throw "Expected error like '$Pattern', received: $($_.Exception.Message)"
|
||||||
|
}
|
||||||
|
throw "Expected error like '$Pattern', but no error was thrown."
|
||||||
|
}
|
||||||
|
|
||||||
|
$generator = Join-Path $Root 'tools/Build-SkillIndex.ps1'
|
||||||
|
$resolver = Join-Path $Root 'tools/Resolve-SkillWorklist.ps1'
|
||||||
|
$indexSchema = Join-Path $Root 'schemas/skill-index.schema.json'
|
||||||
|
$reportSchema = Join-Path $Root 'schemas/findings-report.schema.json'
|
||||||
|
foreach ($path in $generator, $resolver, $indexSchema, $reportSchema) {
|
||||||
|
if (-not (Test-Path -LiteralPath $path -PathType Leaf)) {
|
||||||
|
throw "Required contract file not found: $path"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
$tmp = Join-Path ([IO.Path]::GetTempPath()) ("skillindex_" + [guid]::NewGuid().ToString('N'))
|
||||||
|
New-Item -ItemType Directory -Path $tmp -Force | Out-Null
|
||||||
|
try {
|
||||||
|
$first = Join-Path $tmp 'first.json'
|
||||||
|
$second = Join-Path $tmp 'second.json'
|
||||||
|
& $generator -BCQualityRoot $Root -IndexPath $first | Out-Null
|
||||||
|
& $generator -BCQualityRoot $Root -IndexPath $second | Out-Null
|
||||||
|
|
||||||
|
$normalize = {
|
||||||
|
param([string] $Path)
|
||||||
|
return ((Get-Content -LiteralPath $Path -Raw) -replace '"generatedAt":"[^"]*"', '"generatedAt":"<timestamp>"')
|
||||||
|
}
|
||||||
|
if ((& $normalize $first) -ne (& $normalize $second)) {
|
||||||
|
throw 'Skill index is not deterministic beyond generatedAt.'
|
||||||
|
}
|
||||||
|
|
||||||
|
$raw = Get-Content -LiteralPath $first -Raw
|
||||||
|
if (-not ($raw | Test-Json -SchemaFile $indexSchema -ErrorAction Stop)) {
|
||||||
|
throw 'Generated skill index does not satisfy schemas/skill-index.schema.json.'
|
||||||
|
}
|
||||||
|
|
||||||
|
$index = $raw | ConvertFrom-Json
|
||||||
|
$skills = @($index.skills)
|
||||||
|
if ($index.skillCount -ne $skills.Count) {
|
||||||
|
throw "skillCount is $($index.skillCount), but the index contains $($skills.Count) records."
|
||||||
|
}
|
||||||
|
|
||||||
|
$paths = @($skills.path)
|
||||||
|
$duplicates = @($paths | Group-Object | Where-Object Count -gt 1)
|
||||||
|
if ($duplicates.Count) {
|
||||||
|
throw "Duplicate skill paths: $($duplicates.Name -join ', ')"
|
||||||
|
}
|
||||||
|
foreach ($path in $paths) {
|
||||||
|
if (-not (Test-Path -LiteralPath (Join-Path $Root $path) -PathType Leaf)) {
|
||||||
|
throw "Indexed skill does not exist: $path"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
$expectedLeaves = @(
|
||||||
|
'microsoft/skills/review/al-performance-review.md',
|
||||||
|
'microsoft/skills/review/al-security-review.md',
|
||||||
|
'microsoft/skills/review/al-privacy-review.md',
|
||||||
|
'microsoft/skills/review/al-upgrade-review.md',
|
||||||
|
'microsoft/skills/review/al-style-review.md',
|
||||||
|
'microsoft/skills/review/al-ui-review.md',
|
||||||
|
'microsoft/skills/review/al-error-handling-review.md',
|
||||||
|
'microsoft/skills/review/al-events-review.md',
|
||||||
|
'microsoft/skills/review/al-interfaces-review.md',
|
||||||
|
'microsoft/skills/review/al-breaking-changes-review.md',
|
||||||
|
'microsoft/skills/review/al-web-services-review.md',
|
||||||
|
'microsoft/skills/review/al-testing-review.md',
|
||||||
|
'microsoft/skills/review/al-data-modeling-review.md',
|
||||||
|
'microsoft/skills/review/al-query-review.md',
|
||||||
|
'microsoft/skills/review/al-reporting-review.md',
|
||||||
|
'microsoft/skills/review/al-appsource-review.md',
|
||||||
|
'microsoft/skills/review/al-telemetry-review.md',
|
||||||
|
'microsoft/skills/review/al-scm-review.md',
|
||||||
|
'microsoft/skills/review/al-finance-review.md'
|
||||||
|
)
|
||||||
|
$review = @($skills | Where-Object id -eq 'al-code-review')
|
||||||
|
if ($review.Count -ne 1) {
|
||||||
|
throw "Expected exactly one al-code-review record, found $($review.Count)."
|
||||||
|
}
|
||||||
|
if ((@($review[0].subSkills) -join "`n") -cne ($expectedLeaves -join "`n")) {
|
||||||
|
throw "al-code-review subSkills did not preserve the declared $($expectedLeaves.Count)-leaf order."
|
||||||
|
}
|
||||||
|
foreach ($leafPath in $expectedLeaves) {
|
||||||
|
$leaf = @($skills | Where-Object path -ceq $leafPath)
|
||||||
|
if ($leaf.Count -ne 1 -or @($leaf[0].subSkills).Count -ne 0) {
|
||||||
|
throw "Expected '$leafPath' to resolve to exactly one leaf action skill."
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
$minimalReport = @{
|
||||||
|
skill = @{ id = 'al-style-review'; version = 1 }
|
||||||
|
outcome = 'completed'
|
||||||
|
summary = @{
|
||||||
|
counts = @{ blocker = 0; major = 0; minor = 0; info = 0 }
|
||||||
|
coverage = @{ 'worklist-size' = 0; 'items-evaluated' = 0 }
|
||||||
|
}
|
||||||
|
findings = @()
|
||||||
|
suppressed = @()
|
||||||
|
} | ConvertTo-Json -Depth 8
|
||||||
|
if (-not ($minimalReport | Test-Json -SchemaFile $reportSchema -ErrorAction Stop)) {
|
||||||
|
throw 'Minimal findings report does not satisfy schemas/findings-report.schema.json.'
|
||||||
|
}
|
||||||
|
|
||||||
|
$reviewSkillText = Get-Content -LiteralPath (
|
||||||
|
Join-Path -Path $Root -ChildPath 'microsoft/skills/review/al-code-review.md'
|
||||||
|
) -Raw
|
||||||
|
$reportExamples = [regex]::Matches($reviewSkillText, '(?s)```json\s*(\{.*?\})\s*```')
|
||||||
|
if ($reportExamples.Count -ne 2) {
|
||||||
|
throw "Expected two al-code-review JSON examples, found $($reportExamples.Count)."
|
||||||
|
}
|
||||||
|
foreach ($example in $reportExamples) {
|
||||||
|
if (-not ($example.Groups[1].Value | Test-Json -SchemaFile $reportSchema -ErrorAction Stop)) {
|
||||||
|
throw 'An al-code-review output example does not satisfy schemas/findings-report.schema.json.'
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
$fixtureRoot = Join-Path -Path $tmp -ChildPath 'fixture'
|
||||||
|
$fixtureSkills = Join-Path -Path $fixtureRoot -ChildPath 'microsoft/skills/review'
|
||||||
|
New-Item -ItemType Directory -Path $fixtureSkills -Force | Out-Null
|
||||||
|
$leaf = @'
|
||||||
|
---
|
||||||
|
kind: action-skill
|
||||||
|
id: al-leaf-review
|
||||||
|
version: 1
|
||||||
|
title: Leaf
|
||||||
|
description: Test leaf.
|
||||||
|
inputs: [file-path]
|
||||||
|
outputs: [findings-report]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Leaf
|
||||||
|
|
||||||
|
## Source
|
||||||
|
Source.
|
||||||
|
## Relevance
|
||||||
|
Relevance.
|
||||||
|
## Worklist
|
||||||
|
Worklist.
|
||||||
|
## Action
|
||||||
|
Action.
|
||||||
|
## Output
|
||||||
|
Output.
|
||||||
|
'@
|
||||||
|
Set-Content -LiteralPath (Join-Path $fixtureSkills 'al-leaf-review.md') -Value $leaf -Encoding utf8NoBOM
|
||||||
|
|
||||||
|
$duplicateSuper = @'
|
||||||
|
---
|
||||||
|
kind: action-skill
|
||||||
|
id: al-code-review
|
||||||
|
version: 1
|
||||||
|
title: Review
|
||||||
|
description: Test super-skill.
|
||||||
|
inputs: [file-path]
|
||||||
|
outputs: [findings-report]
|
||||||
|
sub-skills:
|
||||||
|
- microsoft/skills/review/al-leaf-review.md
|
||||||
|
- microsoft/skills/review/al-leaf-review.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# Review
|
||||||
|
|
||||||
|
## Source
|
||||||
|
Source.
|
||||||
|
## Relevance
|
||||||
|
Relevance.
|
||||||
|
## Worklist
|
||||||
|
Worklist.
|
||||||
|
## Action
|
||||||
|
Action.
|
||||||
|
## Output
|
||||||
|
Output.
|
||||||
|
'@
|
||||||
|
$superPath = Join-Path $fixtureSkills 'al-code-review.md'
|
||||||
|
Set-Content -LiteralPath $superPath -Value $duplicateSuper -Encoding utf8NoBOM
|
||||||
|
Assert-ThrowsLike -Pattern '*duplicate sub-skill*' -Action {
|
||||||
|
& $generator -BCQualityRoot $fixtureRoot -IndexPath (Join-Path $tmp 'invalid.json')
|
||||||
|
}
|
||||||
|
|
||||||
|
$nestedLeaf = $leaf.Replace('id: al-leaf-review', 'id: al-nested-review').Replace(
|
||||||
|
'outputs: [findings-report]',
|
||||||
|
"outputs: [findings-report]`nsub-skills:`n - microsoft/skills/review/al-leaf-review.md"
|
||||||
|
)
|
||||||
|
Set-Content -LiteralPath (Join-Path $fixtureSkills 'al-nested-review.md') -Value $nestedLeaf -Encoding utf8NoBOM
|
||||||
|
$nestedSuper = @'
|
||||||
|
---
|
||||||
|
kind: action-skill
|
||||||
|
id: al-code-review
|
||||||
|
version: 1
|
||||||
|
title: Review
|
||||||
|
description: Test super-skill.
|
||||||
|
inputs: [file-path]
|
||||||
|
outputs: [findings-report]
|
||||||
|
sub-skills:
|
||||||
|
- microsoft/skills/review/al-nested-review.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# Review
|
||||||
|
|
||||||
|
## Source
|
||||||
|
Source.
|
||||||
|
## Relevance
|
||||||
|
Relevance.
|
||||||
|
## Worklist
|
||||||
|
Worklist.
|
||||||
|
## Action
|
||||||
|
Action.
|
||||||
|
## Output
|
||||||
|
Output.
|
||||||
|
'@
|
||||||
|
Set-Content -LiteralPath $superPath -Value $nestedSuper -Encoding utf8NoBOM
|
||||||
|
Assert-ThrowsLike -Pattern '*Nested super-skills are not supported*' -Action {
|
||||||
|
& $generator -BCQualityRoot $fixtureRoot -IndexPath (Join-Path $tmp 'nested.json')
|
||||||
|
}
|
||||||
|
|
||||||
|
Remove-Item -LiteralPath (Join-Path $fixtureSkills 'al-nested-review.md') -Force
|
||||||
|
$validSuper = @'
|
||||||
|
---
|
||||||
|
kind: action-skill
|
||||||
|
id: al-code-review
|
||||||
|
version: 1
|
||||||
|
title: Review
|
||||||
|
description: Test super-skill.
|
||||||
|
inputs: [file-path]
|
||||||
|
outputs: [findings-report]
|
||||||
|
sub-skills:
|
||||||
|
- microsoft/skills/review/al-leaf-review.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# Review
|
||||||
|
|
||||||
|
## Source
|
||||||
|
Source.
|
||||||
|
## Relevance
|
||||||
|
Relevance.
|
||||||
|
## Worklist
|
||||||
|
Worklist.
|
||||||
|
## Action
|
||||||
|
Action.
|
||||||
|
## Output
|
||||||
|
Output.
|
||||||
|
'@
|
||||||
|
Set-Content -LiteralPath $superPath -Value $validSuper -Encoding utf8NoBOM
|
||||||
|
|
||||||
|
$customSkills = Join-Path -Path $fixtureRoot -ChildPath 'custom/skills/review'
|
||||||
|
New-Item -ItemType Directory -Path $customSkills -Force | Out-Null
|
||||||
|
$customLeaf = $leaf.Replace('title: Leaf', 'title: Custom Leaf')
|
||||||
|
Set-Content -LiteralPath (Join-Path $customSkills 'custom-leaf-review.md') -Value $customLeaf -Encoding utf8NoBOM
|
||||||
|
|
||||||
|
$layeredPath = Join-Path $tmp 'layered.json'
|
||||||
|
& $generator -BCQualityRoot $fixtureRoot -IndexPath $layeredPath | Out-Null
|
||||||
|
$layeredIndex = Get-Content -LiteralPath $layeredPath -Raw | ConvertFrom-Json
|
||||||
|
$layeredLeaves = @($layeredIndex.skills | Where-Object id -eq 'al-leaf-review')
|
||||||
|
if ($layeredLeaves.Count -ne 2) {
|
||||||
|
throw "Expected both layered al-leaf-review implementations, found $($layeredLeaves.Count)."
|
||||||
|
}
|
||||||
|
if ((@($layeredLeaves.layer | Sort-Object) -join ',') -cne 'custom,microsoft') {
|
||||||
|
throw 'Layered al-leaf-review implementations did not preserve custom and microsoft records.'
|
||||||
|
}
|
||||||
|
|
||||||
|
$resolved = & $resolver -BCQualityRoot $fixtureRoot -IndexPath $layeredPath -SuperSkillPath (
|
||||||
|
'microsoft/skills/review/al-code-review.md'
|
||||||
|
)
|
||||||
|
if ($resolved.subSkills.Count -ne 1 -or
|
||||||
|
$resolved.subSkills[0].path -cne 'custom/skills/review/custom-leaf-review.md') {
|
||||||
|
throw 'The custom implementation did not win the layered leaf slot.'
|
||||||
|
}
|
||||||
|
|
||||||
|
$microsoftOnly = & $resolver -BCQualityRoot $fixtureRoot -IndexPath $layeredPath -SuperSkillPath (
|
||||||
|
'microsoft/skills/review/al-code-review.md'
|
||||||
|
) -EnabledLayers microsoft
|
||||||
|
if ($microsoftOnly.subSkills.Count -ne 1 -or
|
||||||
|
$microsoftOnly.subSkills[0].path -cne 'microsoft/skills/review/al-leaf-review.md') {
|
||||||
|
throw 'Disabling the custom layer did not fall back to the Microsoft implementation.'
|
||||||
|
}
|
||||||
|
|
||||||
|
$customDisabled = & $resolver -BCQualityRoot $fixtureRoot -IndexPath $layeredPath -SuperSkillPath (
|
||||||
|
'microsoft/skills/review/al-code-review.md'
|
||||||
|
) -DisabledSkills 'custom/skills/review/custom-leaf-review.md'
|
||||||
|
if ($customDisabled.subSkills.Count -ne 1 -or
|
||||||
|
$customDisabled.subSkills[0].path -cne 'microsoft/skills/review/al-leaf-review.md') {
|
||||||
|
throw 'Disabling the custom implementation did not fall back to Microsoft.'
|
||||||
|
}
|
||||||
|
|
||||||
|
Set-Content -LiteralPath (Join-Path $customSkills 'duplicate-leaf-review.md') -Value $customLeaf -Encoding utf8NoBOM
|
||||||
|
Assert-ThrowsLike -Pattern '*Duplicate action-skill IDs within a layer: custom:al-leaf-review*' -Action {
|
||||||
|
& $generator -BCQualityRoot $fixtureRoot -IndexPath (Join-Path $tmp 'duplicate-layer.json')
|
||||||
|
}
|
||||||
|
}
|
||||||
|
finally {
|
||||||
|
Remove-Item -LiteralPath $tmp -Recurse -Force -ErrorAction SilentlyContinue
|
||||||
|
}
|
||||||
|
|
||||||
|
Write-Output "Skill-index check PASSED: deterministic, schema-valid, layered overrides resolved, and all $($expectedLeaves.Count) review leaves preserved in order."
|
||||||
51
.github/scripts/validate_frontmatter.py
vendored
51
.github/scripts/validate_frontmatter.py
vendored
|
|
@ -45,7 +45,7 @@ ENTRY_SKILL_REQUIRED_KEYS = {"kind", "id", "version", "title"}
|
||||||
HOST_SKILL_REQUIRED_KEYS = {"name", "description"}
|
HOST_SKILL_REQUIRED_KEYS = {"name", "description"}
|
||||||
|
|
||||||
STANDARD_INPUTS = {
|
STANDARD_INPUTS = {
|
||||||
"pr-diff", "object-list", "file-path", "repository", "telemetry-query",
|
"pr-diff", "object-list", "file-path", "folder-path", "repository", "telemetry-query",
|
||||||
}
|
}
|
||||||
ALLOWED_OUTPUTS = {"findings-report"}
|
ALLOWED_OUTPUTS = {"findings-report"}
|
||||||
VALID_SAMPLE_KINDS = {"good", "bad"}
|
VALID_SAMPLE_KINDS = {"good", "bad"}
|
||||||
|
|
@ -381,6 +381,20 @@ def validate_action_skill(path: Path, parsed: Parsed, report: Report) -> None:
|
||||||
bad = [x for x in ss if not x.endswith(".md")]
|
bad = [x for x in ss if not x.endswith(".md")]
|
||||||
if bad:
|
if bad:
|
||||||
report.error(path, "R20", f"sub-skills entries must end in '.md': {bad}", 1)
|
report.error(path, "R20", f"sub-skills entries must end in '.md': {bad}", 1)
|
||||||
|
non_canonical = [
|
||||||
|
x for x in ss
|
||||||
|
if "\\" in x or x.startswith("/") or ".." in Path(x).parts or x.startswith("./")
|
||||||
|
]
|
||||||
|
if non_canonical:
|
||||||
|
report.error(
|
||||||
|
path,
|
||||||
|
"R20",
|
||||||
|
f"sub-skills entries must be canonical repo-relative paths: {non_canonical}",
|
||||||
|
1,
|
||||||
|
)
|
||||||
|
duplicates = sorted({x for x in ss if ss.count(x) > 1})
|
||||||
|
if duplicates:
|
||||||
|
report.error(path, "R20", f"sub-skills contains duplicate paths: {duplicates}", 1)
|
||||||
|
|
||||||
# R21 five required sections, in order, each exactly once
|
# R21 five required sections, in order, each exactly once
|
||||||
heads = [h for h, _ in headings_in_order(parsed.body)]
|
heads = [h for h, _ in headings_in_order(parsed.body)]
|
||||||
|
|
@ -565,7 +579,13 @@ class SkillRecord:
|
||||||
skill_id: str | None
|
skill_id: str | None
|
||||||
|
|
||||||
|
|
||||||
def validate_sub_skills_registry(path: Path, fm: dict[str, Any], root: Path, report: Report) -> None:
|
def validate_sub_skills_registry(
|
||||||
|
path: Path,
|
||||||
|
fm: dict[str, Any],
|
||||||
|
root: Path,
|
||||||
|
action_skills_by_path: dict[str, dict[str, Any]],
|
||||||
|
report: Report,
|
||||||
|
) -> None:
|
||||||
"""R26: a super-skill's declared `sub-skills` must exactly match the
|
"""R26: a super-skill's declared `sub-skills` must exactly match the
|
||||||
`al-*-review.md` leaf files present in the same directory (set equality,
|
`al-*-review.md` leaf files present in the same directory (set equality,
|
||||||
ordering-agnostic). This keeps the registered leaf list the single source
|
ordering-agnostic). This keeps the registered leaf list the single source
|
||||||
|
|
@ -598,6 +618,17 @@ def validate_sub_skills_registry(path: Path, fm: dict[str, Any], root: Path, rep
|
||||||
f"sub-skills entry is not a sibling 'al-*-review.md' leaf: {entry}", 1,
|
f"sub-skills entry is not a sibling 'al-*-review.md' leaf: {entry}", 1,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
for entry in ss:
|
||||||
|
leaf = action_skills_by_path.get(entry)
|
||||||
|
if leaf is None:
|
||||||
|
if (root / entry).exists():
|
||||||
|
report.error(path, "R26", f"sub-skills entry is not an action skill: {entry}", 1)
|
||||||
|
continue
|
||||||
|
if is_non_empty_list_of_str(leaf.get("sub-skills")):
|
||||||
|
report.error(path, "R26", f"nested super-skill is not permitted in v1 composition: {entry}", 1)
|
||||||
|
if leaf.get("outputs") != ["findings-report"]:
|
||||||
|
report.error(path, "R26", f"sub-skill must produce findings-report: {entry}", 1)
|
||||||
|
|
||||||
# Sibling leaves on disk that were never registered ('forgot to wire it up').
|
# Sibling leaves on disk that were never registered ('forgot to wire it up').
|
||||||
for leaf in sorted(leaves - declared):
|
for leaf in sorted(leaves - declared):
|
||||||
report.error(path, "R26", f"leaf not registered in sub-skills: {leaf}", 1)
|
report.error(path, "R26", f"leaf not registered in sub-skills: {leaf}", 1)
|
||||||
|
|
@ -657,12 +688,16 @@ def run(root: Path) -> Report:
|
||||||
if domain_dir.is_dir():
|
if domain_dir.is_dir():
|
||||||
validate_samples_in_domain(domain_dir, root, report)
|
validate_samples_in_domain(domain_dir, root, report)
|
||||||
|
|
||||||
# Third pass: R24 unique ids within kind
|
# Third pass: R24 unique ids within kind. Layered action-skill overrides
|
||||||
|
# may share an id, but two definitions in one layer are ambiguous.
|
||||||
by_kind: dict[str, dict[str, list[Path]]] = {}
|
by_kind: dict[str, dict[str, list[Path]]] = {}
|
||||||
for rec in skill_records:
|
for rec in skill_records:
|
||||||
if rec.skill_id is None:
|
if rec.skill_id is None:
|
||||||
continue
|
continue
|
||||||
by_kind.setdefault(rec.kind, {}).setdefault(rec.skill_id, []).append(rec.path)
|
scope = rec.kind
|
||||||
|
if rec.kind == "action-skill":
|
||||||
|
scope = f"{rec.kind}:{rec.path.relative_to(root).parts[0]}"
|
||||||
|
by_kind.setdefault(scope, {}).setdefault(rec.skill_id, []).append(rec.path)
|
||||||
for kind, by_id in by_kind.items():
|
for kind, by_id in by_kind.items():
|
||||||
for sid, paths in by_id.items():
|
for sid, paths in by_id.items():
|
||||||
if len(paths) > 1:
|
if len(paths) > 1:
|
||||||
|
|
@ -670,9 +705,13 @@ def run(root: Path) -> Report:
|
||||||
others = [q.relative_to(root).as_posix() for q in paths if q != p]
|
others = [q.relative_to(root).as_posix() for q in paths if q != p]
|
||||||
report.error(p, "R24", f"skill id '{sid}' ({kind}) is not unique; also defined in: {others}")
|
report.error(p, "R24", f"skill id '{sid}' ({kind}) is not unique; also defined in: {others}")
|
||||||
|
|
||||||
# Fourth pass: R26 sub-skills registry matches leaf files on disk
|
# Fourth pass: R26 sub-skills registry matches compatible leaf files on disk
|
||||||
|
action_skills_by_path = {
|
||||||
|
path.relative_to(root).as_posix(): fm
|
||||||
|
for path, fm in action_skill_fms
|
||||||
|
}
|
||||||
for path, fm in action_skill_fms:
|
for path, fm in action_skill_fms:
|
||||||
validate_sub_skills_registry(path, fm, root, report)
|
validate_sub_skills_registry(path, fm, root, action_skills_by_path, report)
|
||||||
|
|
||||||
return report
|
return report
|
||||||
|
|
||||||
|
|
|
||||||
5
.github/workflows/flag-new-top-level.yml
vendored
5
.github/workflows/flag-new-top-level.yml
vendored
|
|
@ -40,11 +40,12 @@ jobs:
|
||||||
// Known, intended repository root. Anything else added at the root
|
// Known, intended repository root. Anything else added at the root
|
||||||
// is flagged for a human to eyeball.
|
// is flagged for a human to eyeball.
|
||||||
const ALLOWED_DIRS = new Set([
|
const ALLOWED_DIRS = new Set([
|
||||||
'.claude-plugin', '.github', 'community', 'custom', 'microsoft', 'skills', 'tools',
|
'.claude-plugin', '.github', 'community', 'custom', 'docs', 'evaluation',
|
||||||
|
'microsoft', 'skills', 'tools',
|
||||||
]);
|
]);
|
||||||
const ALLOWED_FILES = new Set([
|
const ALLOWED_FILES = new Set([
|
||||||
'.gitignore', 'CODEOWNERS', 'LICENSE', 'README.md',
|
'.gitignore', 'CODEOWNERS', 'LICENSE', 'README.md',
|
||||||
'SECURITY.md', 'agent-consumption.md',
|
'SECURITY.md', 'plugin.json',
|
||||||
]);
|
]);
|
||||||
|
|
||||||
const MARKER = '<!-- guard:new-top-level -->';
|
const MARKER = '<!-- guard:new-top-level -->';
|
||||||
|
|
|
||||||
21
.github/workflows/review-fixtures.yml
vendored
21
.github/workflows/review-fixtures.yml
vendored
|
|
@ -12,7 +12,26 @@ jobs:
|
||||||
steps:
|
steps:
|
||||||
- name: Check out repository
|
- name: Check out repository
|
||||||
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||||
|
with:
|
||||||
|
fetch-depth: 0
|
||||||
|
|
||||||
|
- name: Collect changed paths
|
||||||
|
shell: pwsh
|
||||||
|
env:
|
||||||
|
BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }}
|
||||||
|
run: |
|
||||||
|
$paths = if (-not $env:BASE_SHA -or $env:BASE_SHA -match '^0+$') {
|
||||||
|
@(git diff-tree --no-commit-id --name-only -r $env:GITHUB_SHA)
|
||||||
|
} else {
|
||||||
|
@(git diff --name-only $env:BASE_SHA $env:GITHUB_SHA)
|
||||||
|
}
|
||||||
|
$paths | Set-Content -LiteralPath "$env:RUNNER_TEMP/changed-paths.txt" -Encoding utf8NoBOM
|
||||||
|
|
||||||
- name: Validate review evaluation corpus
|
- name: Validate review evaluation corpus
|
||||||
shell: pwsh
|
shell: pwsh
|
||||||
run: ./tools/Test-ReviewFixtures.ps1 -Root . -PrepareDirectory "$env:RUNNER_TEMP/bcquality-review-fixtures"
|
run: |
|
||||||
|
./tools/Test-ReviewFixtures.ps1 -Root . `
|
||||||
|
-PrepareDirectory "$env:RUNNER_TEMP/bcquality-review-fixtures" `
|
||||||
|
-ChangedPathsFile "$env:RUNNER_TEMP/changed-paths.txt" `
|
||||||
|
-CoverageReportPath "$env:RUNNER_TEMP/review-coverage.json"
|
||||||
|
Get-Content -LiteralPath "$env:RUNNER_TEMP/review-coverage.json"
|
||||||
|
|
|
||||||
18
.github/workflows/skill-index.yml
vendored
Normal file
18
.github/workflows/skill-index.yml
vendored
Normal file
|
|
@ -0,0 +1,18 @@
|
||||||
|
name: Validate skill index and report schemas
|
||||||
|
|
||||||
|
on:
|
||||||
|
pull_request:
|
||||||
|
branches: [main]
|
||||||
|
push:
|
||||||
|
branches: [main]
|
||||||
|
|
||||||
|
jobs:
|
||||||
|
validate-contract:
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
steps:
|
||||||
|
- name: Check out repository
|
||||||
|
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||||
|
|
||||||
|
- name: Validate skill-index generator and schemas
|
||||||
|
shell: pwsh
|
||||||
|
run: ./.github/scripts/Test-SkillIndex.ps1 -Root .
|
||||||
258
README.md
258
README.md
|
|
@ -1,207 +1,131 @@
|
||||||
# BCQuality
|
# BC Quality - Don’t teach one agent. Teach the ecosystem. 🤝
|
||||||
|
|
||||||
Quality skills and knowledge for Business Central development.
|
Quality skills and knowledge that help AI tools make better Business Central
|
||||||
|
development decisions: catch BC-specific defects, avoid misleading advice,
|
||||||
|
and explain findings with references you can read.
|
||||||
|
|
||||||
BCQuality is a curated knowledge base and skills library for Business Central. It provides structured, machine-readable guidance that development agents and tools can consume — establishing a consistent quality bar across tooling and teams.
|
BCQuality contains **knowledge and reusable skills**, not agents or a Business
|
||||||
|
Central extension. Your host supplies the agent. You can install the content
|
||||||
|
as a plugin, use it from another integration, or browse the knowledge directly.
|
||||||
|
|
||||||
## What belongs here
|
## Quick start
|
||||||
|
|
||||||
BCQuality is a remedial knowledge base. A file exists because a capable LLM **would get something wrong, or miss something, without it** — not because the topic is important. The admission test for a knowledge file is one question:
|
The walkthrough below uses **GitHub Copilot CLI in a terminal**, not the
|
||||||
|
Copilot Chat panel in VS Code. First
|
||||||
> If this file did not exist, would a modern LLM reviewing or generating BC code make a mistake this file would have prevented?
|
[install Copilot CLI and sign in](https://docs.github.com/en/copilot/get-started/cli-quickstart).
|
||||||
|
Your account and organization policy must allow its use. You do not need to
|
||||||
If the answer is no — the advice is generic software-engineering guidance, or the LLM already knows the BC mechanic in question — the file does not belong here, regardless of how sound the content is. A file earns its place by encoding something BC-specific that LLMs demonstrably get wrong: a CodeCop rule number, a platform API whose semantics the training data gets backwards, a non-obvious ordering rule, a BC property whose default is a footgun.
|
clone BCQuality, build a runner, or deploy an app to Business Central for this
|
||||||
|
source-review example.
|
||||||
Good fit: "`SetLoadFields` must be called before filters, not after" (non-obvious ordering rule). "`FindSet(true)` takes a LockTable and the two-parameter signature is obsolete" (subtle platform behaviour + outdated training data). "CodeCop AA0233 flags `FindFirst … Next` loops" (rule-specific).
|
|
||||||
|
|
||||||
Poor fit: "Use HTTPS instead of HTTP." "Don't hardcode secrets." "Keep transactions short." These are true but any capable LLM already applies them without prompting.
|
|
||||||
|
|
||||||
The practical consequence: when a code-review agent flags something it shouldn't have, or misses something it should have caught, the remedy is a new knowledge file. When it already behaves correctly on a topic, no file is needed.
|
|
||||||
|
|
||||||
A file that *prevents* a false positive — documenting why a pattern is legitimate so the agent stops flagging it — is as valid as one that catches a defect: negative clarifications are first-class knowledge files. What never belongs is a BC fact hard-coded into a skill. Skills are finders and appliers; knowledge files are what the agent knows. See [`skills/do.md`](skills/do.md) and [`skills/write.md`](skills/write.md).
|
|
||||||
|
|
||||||
## What's in this repo
|
|
||||||
|
|
||||||
BCQuality contains **knowledge** and **skills**. It does not contain agents. Agents that consume BCQuality ship with [AL-Go](https://github.com/microsoft/AL-Go) and other orchestrators.
|
|
||||||
|
|
||||||
### 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 three layers:
|
|
||||||
|
|
||||||
- **`/microsoft/`** — Microsoft-endorsed layer.
|
|
||||||
- `/microsoft/knowledge/` — Platform guardrails, official guidance.
|
|
||||||
- `/microsoft/skills/` — Microsoft-endorsed action skills.
|
|
||||||
- **`/community/`** — BC community layer.
|
|
||||||
- `/community/knowledge/` — Community patterns and shared guidance.
|
|
||||||
- `/community/skills/` — Community-contributed action skills.
|
|
||||||
|
|
||||||
- **`/custom/`** — Partner- and customer-specific overrides. Empty by default; populated in forks.
|
|
||||||
- `/custom/knowledge/` — Organization-specific knowledge files.
|
|
||||||
- `/custom/skills/` — Organization-specific action skills.
|
|
||||||
|
|
||||||
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
|
|
||||||
|
|
||||||
Skills define how agents consume knowledge. They come in three flavors:
|
|
||||||
|
|
||||||
- **The entry-point skill** ([`skills/entry.md`](skills/entry.md)) — the first skill an agent invokes at runtime. Given a task context (goal, available inputs, technologies, BC version, etc.), it returns a **dispatch record** naming the action skill or skills to invoke next. Routing logic lives here, not in the orchestrator.
|
|
||||||
|
|
||||||
- **Meta-skill contracts** (`/skills/`) — three stable references that define the rest of the repo:
|
|
||||||
1. **Schema + Use** (READ, [`skills/read.md`](skills/read.md)) — how to read a knowledge file: interpret frontmatter, parse sections, understand layer precedence. Any agent or skill that reads knowledge files depends on it.
|
|
||||||
2. **Action Skill** (DO, [`skills/do.md`](skills/do.md)) — the template every action skill follows. Defines the four-step pattern (Source → Relevance → Worklist → Action) and the structured output format that orchestrators expect.
|
|
||||||
3. **New Knowledge** (WRITE, [`skills/write.md`](skills/write.md)) — how to author a valid knowledge file. References Schema + Use for the format specification and adds authoring rules (atomicity, section guidance).
|
|
||||||
|
|
||||||
READ and DO are read on demand — typically when the first dispatched action skill runs. They are not prerequisites for invoking Entry. WRITE is only used when scaffolding new content.
|
|
||||||
|
|
||||||
- **Action skills** — concrete skills that follow the Action Skill template to do real work (review code, audit telemetry, etc.). Action skills live inside the layers that own them (`/microsoft/skills/`, `/community/skills/`, `/custom/skills/`). An action skill is either a **leaf** that evaluates knowledge files directly, or a **super-skill** that composes other action skills (declared via `sub-skills` in frontmatter). The canonical reference is [`microsoft/skills/review/al-code-review.md`](microsoft/skills/review/al-code-review.md) (super-skill), which composes the AL review leaf skills under [`microsoft/skills/review/`](microsoft/skills/review/) — one per knowledge domain.
|
|
||||||
|
|
||||||
### Agent bootstrapping
|
|
||||||
|
|
||||||
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
|
### Standalone plugin installation
|
||||||
|
|
||||||
BCQuality can also be installed directly as a plugin. The plugin registers one
|
Run these commands in your terminal:
|
||||||
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:
|
```powershell
|
||||||
|
|
||||||
```shell
|
|
||||||
copilot plugin install microsoft/BCQuality
|
copilot plugin install microsoft/BCQuality
|
||||||
|
copilot plugin list
|
||||||
```
|
```
|
||||||
|
|
||||||
Plugin version `0.2.0` renamed the former `bcquality-al-review` skill to
|
The list should include `bcquality`. The plugin currently exposes the
|
||||||
`al-code-review`; explicit invocations and allowlists using the old skill name
|
[`al-code-review`](skills/al-code-review/SKILL.md) skill. Installation and skill
|
||||||
must be updated. The name remains distinct from BC-ALAgents' public
|
discovery are the general pattern; reviewing an app is one example of using it.
|
||||||
`al-review` skill because current hosts may load plugin skill names into one
|
|
||||||
shared inventory.
|
|
||||||
|
|
||||||
The adapter is intentionally not a second review implementation:
|
### Example: Review a complete app folder
|
||||||
|
|
||||||
```text
|
Start a **new** CLI session in your own app folder, replacing the example path:
|
||||||
standalone host skill: skills/al-code-review/SKILL.md
|
|
||||||
-> routing contract: skills/entry.md
|
```powershell
|
||||||
-> review coordinator: microsoft/skills/review/al-code-review.md
|
cd "C:\Repos\MyBusinessCentralApp"
|
||||||
-> domain review leaves
|
copilot
|
||||||
```
|
```
|
||||||
|
|
||||||
Only the first file follows the host's `SKILL.md` packaging format. The
|
Approve access only to a project you trust, then ask:
|
||||||
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`
|
> Use the installed al-code-review skill to review the complete Business Central
|
||||||
narrows discovery without removing any files. Layer selection is a filter here,
|
> app in this folder without changing my source files. Return the complete
|
||||||
not a deny mechanism — see [the adapter](skills/al-code-review/SKILL.md) for the
|
> BCQuality findings report.
|
||||||
difference from the pruned-clone model.
|
|
||||||
|
|
||||||
The host adapter and internal action skill intentionally share the
|
The folder should contain `app.json` and your AL source; it does **not** need
|
||||||
`al-code-review` name: they expose the same operation in two different skill
|
to be a Git repository. On macOS or Linux, use your app's local path instead.
|
||||||
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
|
Expect a report for each selected review, with findings, source locations,
|
||||||
|
severity, confidence, and references to the relevant guidance. Some hosts show
|
||||||
|
the structured JSON directly. `completed` with no findings means nothing was
|
||||||
|
flagged in that review's scope; `partial` or `failed` is **not** a clean result.
|
||||||
|
See [reading your results](docs/using-bcquality.md#reading-your-results).
|
||||||
|
|
||||||
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.
|
[PowerShell 7](https://learn.microsoft.com/en-us/powershell/scripting/install/installing-powershell)
|
||||||
|
(`pwsh`) is recommended for fast knowledge discovery. If it is unavailable,
|
||||||
|
the review can still discover knowledge by reading the folders.
|
||||||
|
|
||||||
### Frontmatter schema (v1)
|
## Documentation
|
||||||
|
|
||||||
```yaml
|
| I want to... | Start here |
|
||||||
---
|
| --- | --- |
|
||||||
bc-version: [all] # or [26..28], or [26..] for "26 and later"
|
| Choose direct reading, a supplied skill, or my own agent | [Ways to use BCQuality](docs/using-bcquality.md#choose-how-to-use-bcquality) |
|
||||||
domain: performance # security | performance | ux | telemetry | ...
|
| Review a file, changes, a branch, or a particular concern | [Using BCQuality](docs/using-bcquality.md) |
|
||||||
keywords: [query, filtering, partial] # free-text tags for retrieval
|
| Resolve setup problems, incomplete reviews, or incorrect findings | [Troubleshooting and support](docs/troubleshooting.md) |
|
||||||
technologies: [al] # al | javascript | powershell | ...
|
| Browse the available guidance | [Knowledge by domain](docs/using-bcquality.md#knowledge-by-domain) |
|
||||||
countries: [w1] # ISO codes, or [w1]
|
| Configure the plugin or use my organization's rules | [Customizing BCQuality](docs/customizing-bcquality.md) |
|
||||||
application-area: [all] # finance | manufacturing | jobs | [all]
|
| Contribute knowledge or improve a rule | [Your first contribution](docs/contributing.md#your-first-contribution) |
|
||||||
---
|
| Connect a host, agent, or CI integration | [Minimal integration example](docs/agent-consumption.md#try-a-minimal-integration) |
|
||||||
```
|
|
||||||
|
|
||||||
All six fields are required. The schema is locked — changes require a PR approved by both maintainers.
|
[All documentation and technical references](docs/README.md).
|
||||||
|
|
||||||
### Sections
|
|
||||||
|
|
||||||
Every knowledge file must contain a `## Description` section. The following sections are optional but recommended:
|
|
||||||
|
|
||||||
- **`## Best Practice`** — the recommended approach
|
|
||||||
- **`## Anti Pattern`** — what to avoid and why
|
|
||||||
|
|
||||||
Code examples belong in separate files, not in the knowledge file itself. Knowledge files must not contain fenced code blocks.
|
|
||||||
|
|
||||||
## Scope
|
## Scope
|
||||||
|
|
||||||
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.
|
Today's curated content covers **technical AL code review** and a focused
|
||||||
|
**Supply Chain Management (SCM)** functional domain. It augments
|
||||||
|
the agent's judgment; it is not an exhaustive BC manual or a substitute for
|
||||||
|
compilation, analyzers, tests, or human review. See
|
||||||
|
[coverage and limits](docs/using-bcquality.md#coverage-and-limits) for the
|
||||||
|
available domains and the difference between a folder review and a comparison.
|
||||||
|
Mechanical issues already enforced by the AL compiler or standard analyzers are
|
||||||
|
intentionally left to those deterministic tools rather than duplicated here.
|
||||||
|
|
||||||
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.
|
The [SCM domain](microsoft/knowledge/scm/) covers selected inventory, costing,
|
||||||
|
reservation, tracking, and warehouse/posting workflows, not exhaustive supply
|
||||||
|
chain validation. Broader functional coverage such as Finance, Manufacturing,
|
||||||
|
Jobs, and Service, and technologies such as PowerShell, pipelines, and Power
|
||||||
|
Platform, remain valid future scope, **not current coverage claims**.
|
||||||
|
|
||||||
## How agents consume BCQuality
|
## What's in this repo
|
||||||
|
|
||||||
Action skills follow a four-step pattern:
|
Knowledge articles cover one concern each. Skills tell an agent how to find
|
||||||
|
and apply the relevant knowledge. Both live in three layers:
|
||||||
|
|
||||||
1. **Source** — which knowledge folders and tags to search
|
| Layer | Purpose |
|
||||||
2. **Relevance** — filter by frontmatter (version, technology, country, area)
|
| --- | --- |
|
||||||
3. **Worklist** — narrow from N candidates to the M that apply to the current task
|
| [Microsoft](microsoft/) | Microsoft-endorsed skills and their knowledge. |
|
||||||
4. **Action** — apply the relevant knowledge and produce structured output
|
| [Community](community/) | Community-owned skills and their knowledge. |
|
||||||
|
| [Custom](custom/) | Organization-specific additions and overrides in your own fork. |
|
||||||
|
|
||||||
Every action skill produces output in a common format that orchestrators can consume without skill-specific parsing. The format is JSON and includes an `outcome` (so a clean run, a not-applicable skill, and a partial failure are all distinguishable), `findings` (what the skill observed), structured `references` back to the knowledge files that informed each finding, per-finding `confidence`, and a `suppressed` list recording any knowledge files overridden by layer precedence. This contract is defined in the Action Skill meta-skill so that orchestrators and action skills remain independently evolvable.
|
All three are enabled by default; Custom is empty upstream. You do not need
|
||||||
|
to configure layers to get started.
|
||||||
BCQuality is an **additive** knowledge layer: it augments the agent's review judgement, it does not replace it. Super-skills (such as `al-code-review`) run a self-review pass alongside their sub-skills and surface concerns the agent identified on its own, marked with `from-sub-skill: "agent"` and an empty `references: []` so consumers can render them distinctly from knowledge-backed findings. See [agent-consumption.md](agent-consumption.md) and [`skills/do.md`](skills/do.md) for the full contract.
|
|
||||||
|
|
||||||
The meta-skills in `/skills/` define this pattern. Every concrete action skill follows it.
|
|
||||||
|
|
||||||
For the end-to-end flow — from orchestrator trigger through to how output reaches developers — see [agent-consumption.md](agent-consumption.md).
|
|
||||||
|
|
||||||
## Repository structure
|
|
||||||
|
|
||||||
```
|
|
||||||
├── /skills/ # Global: entry-point skill + meta-skill contracts (READ, DO, WRITE)
|
|
||||||
├── /evaluation/ # Neutral good/bad review fixtures and scoring contract
|
|
||||||
├── /.github/ # Actions and workflows
|
|
||||||
├── /microsoft/ # Microsoft-endorsed layer
|
|
||||||
│ ├── /knowledge/ # Knowledge files by domain
|
|
||||||
│ │ └── /<domain>/ # Each article: <slug>.md + optional <slug>.good.al / <slug>.bad.al
|
|
||||||
│ └── /skills/ # Microsoft-endorsed action skills
|
|
||||||
├── /community/ # BC community layer
|
|
||||||
│ ├── /knowledge/ # Knowledge files by domain
|
|
||||||
│ │ └── /<domain>/ # Article + sibling samples, same convention
|
|
||||||
│ └── /skills/ # Community action skills
|
|
||||||
├── /custom/ # Partner/customer-specific overrides (empty; populated in forks)
|
|
||||||
│ ├── /knowledge/
|
|
||||||
│ └── /skills/
|
|
||||||
```
|
|
||||||
|
|
||||||
## Versioning
|
## Versioning
|
||||||
|
|
||||||
BCQuality content is released on demand — roughly monthly, not on every commit. A
|
Update the installed plugin from your terminal, then start a new session:
|
||||||
release is a `major.minor` value derived from git tags, cut manually via the
|
|
||||||
`Release version` workflow: pick whether to bump the minor or the major, and it
|
|
||||||
computes the next version and tags the current `main` as `v{major}.{minor}`.
|
|
||||||
|
|
||||||
- Bump the **minor** for the usual periodic content update; bump the **major**
|
```powershell
|
||||||
only for a breaking change.
|
copilot plugin update bcquality
|
||||||
- The minor is a **monotonic counter** — it only ever increments and never
|
```
|
||||||
resets, even across a major bump — so it uniquely identifies a release.
|
|
||||||
|
Plugin versions and content-release tags are different. For reproducible runs
|
||||||
|
and organization forks, see [updates and versions](docs/customizing-bcquality.md#updates-and-versions).
|
||||||
|
|
||||||
|
## What belongs here
|
||||||
|
|
||||||
|
Knowledge belongs here when it prevents a BC-specific mistake an otherwise
|
||||||
|
capable agent would make, including false-positive findings. BC facts belong
|
||||||
|
in knowledge articles, not skill instructions. See the
|
||||||
|
[admission test and examples](docs/contributing.md#what-belongs-here).
|
||||||
|
|
||||||
## Contributing
|
## Contributing
|
||||||
|
|
||||||
Contributions are welcome. Before submitting a PR:
|
Partners are welcome to contribute to the layer that owns the domain,
|
||||||
|
regardless of affiliation. Start with the [contribution guide](docs/contributing.md).
|
||||||
1. Read the knowledge file format above — frontmatter and sections are validated by CI.
|
To report a problem without authoring a rule, see [support](docs/troubleshooting.md#reporting-a-problem).
|
||||||
2. Keep files atomic: one concern per file, under 100 lines.
|
|
||||||
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.
|
|
||||||
|
|
||||||
Companion samples must be referenced by filename from their article, and every referenced sample must exist. The review evaluation corpus under [`evaluation/`](evaluation/) adds one positive and one clean control for every registered AL review leaf; see [`evaluation/README.md`](evaluation/README.md) for credential-free validation and optional fast-model scoring.
|
|
||||||
|
|
||||||
## License
|
## License
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,13 +17,13 @@ An agent is a user, but it cannot configure users or other agents, and it cannot
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`agent-permissions-intersect-with-assigner.good.al`](agent-permissions-intersect-with-assigner.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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 sample: [`agent-permissions-intersect-with-assigner.bad.al`](agent-permissions-intersect-with-assigner.bad.al).
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,13 +17,13 @@ The agent only sees what its profile shows. Extra actions, views, and Role Cente
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`agent-profile-narrows-visible-ui.good.al`](agent-profile-narrows-visible-ui.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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 sample: [`agent-profile-narrows-visible-ui.bad.al`](agent-profile-narrows-visible-ui.bad.al).
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ Instance setup is not a Card or StandardDialog. The toolkit expects `PageType =
|
||||||
|
|
||||||
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`.
|
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`.
|
See sample: [`agent-setup-page-is-configuration-dialog.good.al`](agent-setup-page-is-configuration-dialog.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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`.
|
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`.
|
See sample: [`agent-setup-page-is-configuration-dialog.bad.al`](agent-setup-page-is-configuration-dialog.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,13 +17,13 @@ ConfigurationDialog setup is a draft: the user can Cancel without writing. That
|
||||||
|
|
||||||
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`.
|
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`.
|
See sample: [`agent-setup-source-table-is-temporary.good.al`](agent-setup-source-table-is-temporary.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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 sample: [`agent-setup-source-table-is-temporary.bad.al`](agent-setup-source-table-is-temporary.bad.al).
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ Each agent instance is a user. Instance-specific setup is keyed by that user's `
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`agent-setup-table-keyed-by-user-security-id.good.al`](agent-setup-table-keyed-by-user-security-id.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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`.
|
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`.
|
See sample: [`agent-setup-table-keyed-by-user-security-id.bad.al`](agent-setup-table-keyed-by-user-security-id.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ application-area: [all]
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`analyze-message-error-stops-warning-forces-review.good.al`](analyze-message-error-stops-warning-forces-review.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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`.
|
See sample: [`analyze-message-error-stops-warning-forces-review.bad.al`](analyze-message-error-stops-warning-forces-review.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ Page-filter tweaks, extra validation, and prompt dialogs for the agent should no
|
||||||
|
|
||||||
On `OnAfterInitialization`, exit unless `Agent Session.IsAgentSession`. Then `BindSubscription` a single-instance codeunit that holds the current task id. Keep those subscribers internal.
|
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`.
|
See sample: [`bind-agent-subscribers-only-in-agent-session.good.al`](bind-agent-subscribers-only-in-agent-session.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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`.
|
See sample: [`bind-agent-subscribers-only-in-agent-session.bad.al`](bind-agent-subscribers-only-in-agent-session.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ For isolation, `Agent`, `Agent Task Builder`, and related toolkit codeunits erro
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`cross-app-agent-calls-need-your-public-api.good.al`](cross-app-agent-calls-need-your-public-api.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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`.
|
See sample: [`cross-app-agent-calls-need-your-public-api.bad.al`](cross-app-agent-calls-need-your-public-api.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ application-area: [all]
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`do-not-create-agents-in-install-upgrade-or-background.good.al`](do-not-create-agents-in-install-upgrade-or-background.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
`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`.
|
See sample: [`do-not-create-agents-in-install-upgrade-or-background.bad.al`](do-not-create-agents-in-install-upgrade-or-background.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,13 +17,13 @@ application-area: [all]
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`get-default-access-controls-least-privilege.good.al`](get-default-access-controls-least-privilege.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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 sample: [`get-default-access-controls-least-privilege.bad.al`](get-default-access-controls-least-privilege.bad.al).
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,13 +17,13 @@ application-area: [all]
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`get-default-profile-lives-in-the-app.good.al`](get-default-profile-lives-in-the-app.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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 sample: [`get-default-profile-lives-in-the-app.bad.al`](get-default-profile-lives-in-the-app.bad.al).
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,13 +17,13 @@ The runtime treats instructions as the agent's standing prompt. A one-line goal
|
||||||
|
|
||||||
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`.
|
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`.
|
See sample: [`instruction-structure-is-role-rules-steps.good.al`](instruction-structure-is-role-rules-steps.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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 sample: [`instruction-structure-is-role-rules-steps.bad.al`](instruction-structure-is-role-rules-steps.bad.al).
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,13 +17,13 @@ Agent tools are the UI the profile exposes. Action names and tool ids change acr
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`instructions-describe-work-not-tool-ids.good.al`](instructions-describe-work-not-tool-ids.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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 sample: [`instructions-describe-work-not-tool-ids.bad.al`](instructions-describe-work-not-tool-ids.bad.al).
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ Static instructions stored as an app resource are copied onto an instance only w
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`reapply-resource-instructions-on-upgrade.good.al`](reapply-resource-instructions-on-upgrade.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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`.
|
See sample: [`reapply-resource-instructions-on-upgrade.bad.al`](reapply-resource-instructions-on-upgrade.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,13 +17,13 @@ Each agent type needs a `Copilot Capability` enum value that the factory links a
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`register-copilot-capability-for-the-agent.good.al`](register-copilot-capability-for-the-agent.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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 sample: [`register-copilot-capability-for-the-agent.bad.al`](register-copilot-capability-for-the-agent.bad.al).
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ Instructions are instance data, not an enum caption. `Agent.SetInstructions` tak
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`set-instructions-as-secrettext.good.al`](set-instructions-as-secrettext.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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`.
|
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`.
|
See sample: [`set-instructions-as-secrettext.bad.al`](set-instructions-as-secrettext.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ application-area: [all]
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`show-can-create-agent-does-not-block-code-create.good.al`](show-can-create-agent-does-not-block-code-create.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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`.
|
See sample: [`show-can-create-agent-does-not-block-code-create.bad.al`](show-can-create-agent-does-not-block-code-create.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ Incoming task messages default to requiring user approval before the agent runs.
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`skip-incoming-review-only-for-trusted-input.good.al`](skip-incoming-review-only-for-trusted-input.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
`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`.
|
See sample: [`skip-incoming-review-only-for-trusted-input.bad.al`](skip-incoming-review-only-for-trusted-input.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,13 +17,13 @@ The agent runtime looks for specific phrases: ask for assistance, request a revi
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`use-documented-instruction-keywords.good.al`](use-documented-instruction-keywords.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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.
|
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 sample: [`use-documented-instruction-keywords.bad.al`](use-documented-instruction-keywords.bad.al).
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,13 +17,13 @@ An AL agent type is registered by extending `Agent Metadata Provider`. The platf
|
||||||
|
|
||||||
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.
|
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`.
|
See sample: [`wire-all-three-agent-interfaces.good.al`](wire-all-three-agent-interfaces.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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`.
|
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 sample: [`wire-all-three-agent-interfaces.bad.al`](wire-all-three-agent-interfaces.bad.al).
|
||||||
|
|
||||||
## See also
|
## See also
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,12 @@
|
||||||
|
codeunit 50100 "Rental Profile Install"
|
||||||
|
{
|
||||||
|
Subtype = Install;
|
||||||
|
|
||||||
|
trigger OnInstallAppPerDatabase()
|
||||||
|
var
|
||||||
|
RentalProfile: Record Profile;
|
||||||
|
begin
|
||||||
|
RentalProfile.Init();
|
||||||
|
RentalProfile.Insert(true);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,6 @@
|
||||||
|
profile "RENTAL MANAGER"
|
||||||
|
{
|
||||||
|
Caption = 'Rental Manager';
|
||||||
|
Description = 'Manages rental agreements and equipment availability.';
|
||||||
|
RoleCenter = "Business Manager Role Center";
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: appsource
|
||||||
|
keywords: [profile-object, profile-table, install-codeunit, role-center, page-customization]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Define profiles as AL objects
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Profiles delivered by a Marketplace extension must be declared as AL `profile` objects. A profile object is validated with its Role Center and page customizations when the extension is compiled and is registered through extension synchronization. Inserting profile-table records from install or setup code bypasses that object lifecycle.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Declare each app-owned profile with the `profile` object and set its `RoleCenter`, user-facing caption, and optional customizations in AL. Let installation and synchronization register the object.
|
||||||
|
|
||||||
|
See sample: [`define-profiles-as-al-objects.good.al`](define-profiles-as-al-objects.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Install, upgrade, or setup code that creates an app-owned profile by inserting a `Profile` table record. Detection signal: a `Record Profile` variable followed by `Insert` in profile provisioning code.
|
||||||
|
|
||||||
|
See sample: [`define-profiles-as-al-objects.bad.al`](define-profiles-as-al-objects.bad.al).
|
||||||
|
|
@ -0,0 +1,7 @@
|
||||||
|
codeunit 50100 "Rental Audit"
|
||||||
|
{
|
||||||
|
procedure SetCreatedAt(var RentalAgreement: Record "Rental Agreement")
|
||||||
|
begin
|
||||||
|
RentalAgreement."Created At" := CurrentDateTime() + 7200000;
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,7 @@
|
||||||
|
codeunit 50100 "Rental Audit"
|
||||||
|
{
|
||||||
|
procedure SetCreatedAt(var RentalAgreement: Record "Rental Agreement")
|
||||||
|
begin
|
||||||
|
RentalAgreement."Created At" := CurrentDateTime();
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: appsource
|
||||||
|
keywords: [datetime, time-zone, utc, currentdatetime, locale, regional-settings]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Do not hard-code time-zone offsets
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Marketplace extensions run for users and services in many time zones. Adding a fixed offset to a `DateTime` assumes one locale, ignores daylight-saving transitions, and changes an absolute timestamp into an incorrect value for other regions.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Store and compare `DateTime` values without a manually applied regional offset. Business Central stores `DateTime` values in UTC and presents them according to the client time zone. Keep service contracts time-zone explicit and perform a conversion only when the business requirement identifies a particular zone.
|
||||||
|
|
||||||
|
See sample: [`do-not-hard-code-time-zone-offsets.good.al`](do-not-hard-code-time-zone-offsets.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Adding or subtracting a fixed duration solely to convert `CurrentDateTime` or another timestamp to an assumed local time. Detection signals include fixed hour-sized millisecond values near `DateTime` assignments and comments naming a specific time zone; confirm the duration is an offset rather than a legitimate deadline or schedule interval.
|
||||||
|
|
||||||
|
See sample: [`do-not-hard-code-time-zone-offsets.bad.al`](do-not-hard-code-time-zone-offsets.bad.al).
|
||||||
|
|
@ -0,0 +1,21 @@
|
||||||
|
codeunit 50100 "Rental Service"
|
||||||
|
{
|
||||||
|
[ServiceEnabled]
|
||||||
|
procedure CloseAgreement(AgreementNo: Code[20]): Boolean
|
||||||
|
var
|
||||||
|
RentalAgreement: Record "Rental Agreement";
|
||||||
|
begin
|
||||||
|
if not Confirm(CloseAgreementQst, false, AgreementNo) then
|
||||||
|
exit(false);
|
||||||
|
|
||||||
|
RentalAgreement.Get(AgreementNo);
|
||||||
|
RentalAgreement.Closed := true;
|
||||||
|
RentalAgreement.Modify(true);
|
||||||
|
Message(AgreementClosedMsg, AgreementNo);
|
||||||
|
exit(true);
|
||||||
|
end;
|
||||||
|
|
||||||
|
var
|
||||||
|
CloseAgreementQst: Label 'Close rental agreement %1?';
|
||||||
|
AgreementClosedMsg: Label 'Rental agreement %1 was closed.';
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,15 @@
|
||||||
|
codeunit 50100 "Rental Service"
|
||||||
|
{
|
||||||
|
[ServiceEnabled]
|
||||||
|
procedure CloseAgreement(AgreementNo: Code[20]): Boolean
|
||||||
|
var
|
||||||
|
RentalAgreement: Record "Rental Agreement";
|
||||||
|
begin
|
||||||
|
if not RentalAgreement.Get(AgreementNo) then
|
||||||
|
exit(false);
|
||||||
|
|
||||||
|
RentalAgreement.Closed := true;
|
||||||
|
RentalAgreement.Modify(true);
|
||||||
|
exit(true);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: appsource
|
||||||
|
keywords: [web-service, serviceenabled, guiallowed, message, confirm, strmenu]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Keep web-service paths free of UI calls
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Pages and codeunits exposed as web services run without an interactive client. Calls that require a UI callback, including `Confirm`, `StrMenu`, and modal pages, can terminate the service request instead of completing the operation. `Message` does not raise the callback error: the message is suppressed and logged, making it ineffective for communicating a service result.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Keep service entry points and every procedure they call free of interactive UI. Return data through the service contract and report validation failures with service-safe error handling. When a procedure is shared with an interactive client, guard UI-only behavior with `GuiAllowed` while preserving the underlying operation.
|
||||||
|
|
||||||
|
See sample: [`keep-web-service-paths-free-of-ui-calls.good.al`](keep-web-service-paths-free-of-ui-calls.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
A web-service-exposed page or codeunit calls an interactive UI method directly or indirectly. Detection signals include `Message`, `Confirm`, `StrMenu`, `Page.RunModal`, and confirmation-dialog pages on a service call path. Treat `Message` as suppressed and ineffective, not as a callback failure. Do not flag a controlled `Error` solely because it returns a service fault.
|
||||||
|
|
||||||
|
See sample: [`keep-web-service-paths-free-of-ui-calls.bad.al`](keep-web-service-paths-free-of-ui-calls.bad.al).
|
||||||
|
|
@ -0,0 +1,15 @@
|
||||||
|
pageextension 50100 "Rental Customer List" extends "Customer List"
|
||||||
|
{
|
||||||
|
actions
|
||||||
|
{
|
||||||
|
addafter("Customer Ledger Entries")
|
||||||
|
{
|
||||||
|
action(OpenRentalAgreements)
|
||||||
|
{
|
||||||
|
ApplicationArea = All;
|
||||||
|
Caption = 'Rental Agreements';
|
||||||
|
RunObject = page "Rental Agreement List";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,15 @@
|
||||||
|
pageextension 50100 "Rental Customer List" extends "Customer List"
|
||||||
|
{
|
||||||
|
actions
|
||||||
|
{
|
||||||
|
addlast(Processing)
|
||||||
|
{
|
||||||
|
action(OpenRentalAgreements)
|
||||||
|
{
|
||||||
|
ApplicationArea = All;
|
||||||
|
Caption = 'Rental Agreements';
|
||||||
|
RunObject = page "Rental Agreement List";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: appsource
|
||||||
|
keywords: [pageextension, actions, addfirst, addlast, addbefore, addafter]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Place page extension actions with addfirst or addlast
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Place new page-extension actions at the beginning or end of an existing action group with `addfirst` or `addlast`. Anchoring a new action relative to a specific base-app action with `addbefore` or `addafter` couples the extension to an implementation detail that can move or disappear between Business Central releases.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Choose the semantic action area or group and append or prepend the extension's actions. This keeps placement deterministic without depending on the continued existence of one neighboring action.
|
||||||
|
|
||||||
|
See sample: [`place-page-extension-actions-with-addfirst-or-addlast.good.al`](place-page-extension-actions-with-addfirst-or-addlast.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Using `addbefore` or `addafter` to place newly added actions next to a specific action from another app. The syntax is valid AL, but the placement anchor is brittle for a Marketplace extension.
|
||||||
|
|
||||||
|
See sample: [`place-page-extension-actions-with-addfirst-or-addlast.bad.al`](place-page-extension-actions-with-addfirst-or-addlast.bad.al).
|
||||||
|
|
@ -0,0 +1,20 @@
|
||||||
|
page 50100 "Rental Agreement List"
|
||||||
|
{
|
||||||
|
PageType = List;
|
||||||
|
SourceTable = "Rental Agreement";
|
||||||
|
ApplicationArea = All;
|
||||||
|
|
||||||
|
layout
|
||||||
|
{
|
||||||
|
area(Content)
|
||||||
|
{
|
||||||
|
repeater(Agreements)
|
||||||
|
{
|
||||||
|
field("No."; Rec."No.")
|
||||||
|
{
|
||||||
|
ApplicationArea = All;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,21 @@
|
||||||
|
page 50100 "Rental Agreement List"
|
||||||
|
{
|
||||||
|
PageType = List;
|
||||||
|
SourceTable = "Rental Agreement";
|
||||||
|
ApplicationArea = All;
|
||||||
|
UsageCategory = Lists;
|
||||||
|
|
||||||
|
layout
|
||||||
|
{
|
||||||
|
area(Content)
|
||||||
|
{
|
||||||
|
repeater(Agreements)
|
||||||
|
{
|
||||||
|
field("No."; Rec."No.")
|
||||||
|
{
|
||||||
|
ApplicationArea = All;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: appsource
|
||||||
|
keywords: [usagecategory, tell-me, search, page, report, discoverability]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Set UsageCategory on searchable entry points
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Pages and reports that users are expected to open directly must set `UsageCategory`. Without it, the object is absent from Tell Me and users cannot bookmark it from the web client. Supporting objects such as list parts, dialogs, API pages, and objects reached only through another page do not need to be searchable entry points.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Set `UsageCategory` to the category that matches the entry point, such as `Lists`, `Tasks`, `ReportsAndAnalysis`, or `Documents`. Also set the appropriate object-level `ApplicationArea` so search results respect feature visibility.
|
||||||
|
|
||||||
|
See sample: [`set-usagecategory-on-searchable-entry-points.good.al`](set-usagecategory-on-searchable-entry-points.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
A user-facing page or report intended for direct discovery omits `UsageCategory` or sets it to `None`. Do not infer intent from the object type alone; require evidence that the object is a direct user entry point.
|
||||||
|
|
||||||
|
See sample: [`set-usagecategory-on-searchable-entry-points.bad.al`](set-usagecategory-on-searchable-entry-points.bad.al).
|
||||||
|
|
@ -0,0 +1,10 @@
|
||||||
|
codeunit 50100 "Rental Period Defaults"
|
||||||
|
{
|
||||||
|
procedure GetPolicyStartDate(): Date
|
||||||
|
var
|
||||||
|
PolicyStartDate: Date;
|
||||||
|
begin
|
||||||
|
Evaluate(PolicyStartDate, '01/31/2025');
|
||||||
|
exit(PolicyStartDate);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,7 @@
|
||||||
|
codeunit 50100 "Rental Period Defaults"
|
||||||
|
{
|
||||||
|
procedure GetPolicyStartDate(): Date
|
||||||
|
begin
|
||||||
|
exit(20250131D);
|
||||||
|
end;
|
||||||
|
}
|
||||||
26
community/knowledge/appsource/use-invariant-date-literals.md
Normal file
26
community/knowledge/appsource/use-invariant-date-literals.md
Normal file
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: appsource
|
||||||
|
keywords: [date-literal, invariant-date, dateformula, localization, appsourcecop]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Use invariant date literals
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Write fixed dates in AL with the invariant `yyyymmddD` syntax. A locale-dependent text value parsed with `Evaluate` can change meaning or fail under another user's regional settings, which makes the Marketplace extension unreliable across markets.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Represent a fixed date directly as an AL date literal, such as `20250131D`. Use `CalcDate` with a date formula when the value is relative rather than fixed.
|
||||||
|
|
||||||
|
See sample: [`use-invariant-date-literals.good.al`](use-invariant-date-literals.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Building a fixed date by passing localized text such as `01/02/2025` to `Evaluate`. Detection signal: `Evaluate` converting a hard-coded or label-backed formatted string into a `Date`.
|
||||||
|
|
||||||
|
See sample: [`use-invariant-date-literals.bad.al`](use-invariant-date-literals.bad.al).
|
||||||
|
|
@ -4,7 +4,7 @@ id: al-agents-review
|
||||||
version: 1
|
version: 1
|
||||||
title: AL agents review
|
title: AL agents review
|
||||||
description: Reviews AL source changes against agent guidance from BCQuality.
|
description: Reviews AL source changes against agent guidance from BCQuality.
|
||||||
inputs: [pr-diff, file-path]
|
inputs: [pr-diff, file-path, folder-path]
|
||||||
outputs: [findings-report]
|
outputs: [findings-report]
|
||||||
bc-version: [all]
|
bc-version: [all]
|
||||||
technologies: [al]
|
technologies: [al]
|
||||||
|
|
@ -18,7 +18,10 @@ Reviews AL source changes against the `agents` knowledge domain in BCQuality and
|
||||||
|
|
||||||
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.
|
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.
|
An orchestrator invokes this skill with a `pr-diff`, `file-path`, or
|
||||||
|
`folder-path`. For a folder, review all relevant source below it under DO's
|
||||||
|
current-state input semantics. The skill produces one JSON document
|
||||||
|
conforming to the DO output contract.
|
||||||
|
|
||||||
## Source
|
## Source
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -12,6 +12,17 @@ custom/
|
||||||
|
|
||||||
## How to use
|
## How to use
|
||||||
|
|
||||||
Fork or clone BCQuality into your own repository and add your content here. Knowledge files in `/custom/knowledge/` follow the same frontmatter schema and section requirements as every other layer. Action skills in `/custom/skills/` follow the Action Skill template defined in `/skills/`.
|
Use a fork or organization-controlled copy of BCQuality, not the upstream
|
||||||
|
repository or your AL app's source folder. Confirm `git remote get-url origin`
|
||||||
|
points at your repository before adding custom content. Upstream does not
|
||||||
|
accept custom rules.
|
||||||
|
|
||||||
When agents consume BCQuality, the custom layer is loaded alongside Microsoft and Community — your overrides apply automatically.
|
Follow [Customizing BCQuality](../docs/customizing-bcquality.md) for a worked
|
||||||
|
rule, plugin configuration, installing your fork, and keeping it up to date.
|
||||||
|
Adding a rule here does not update an existing upstream plugin installation;
|
||||||
|
your host must consume your copy.
|
||||||
|
|
||||||
|
Knowledge files follow [READ](../skills/read.md) and action skills follow
|
||||||
|
[DO](../skills/do.md). With the Custom layer enabled, applicable custom
|
||||||
|
knowledge overrides contradictory Community or Microsoft guidance. The report
|
||||||
|
records the displaced article; non-conflicting guidance remains additive.
|
||||||
|
|
|
||||||
34
docs/README.md
Normal file
34
docs/README.md
Normal file
|
|
@ -0,0 +1,34 @@
|
||||||
|
# BCQuality documentation
|
||||||
|
|
||||||
|
**New to BCQuality? Start with the [quick start](../README.md#quick-start).**
|
||||||
|
Install the plugin, discover its skills, and try an app review. No knowledge
|
||||||
|
of BCQuality's internal protocol is needed.
|
||||||
|
|
||||||
|
## Partner guides
|
||||||
|
|
||||||
|
| Goal | Guide |
|
||||||
|
| --- | --- |
|
||||||
|
| Choose direct reading, a supplied skill, or my own agent | [Ways to use BCQuality](using-bcquality.md#choose-how-to-use-bcquality) |
|
||||||
|
| Review an app, file, changes, or branch | [Using BCQuality](using-bcquality.md) |
|
||||||
|
| Understand a report and its limitations | [Reading your results](using-bcquality.md#reading-your-results) |
|
||||||
|
| Find a particular rule or example | [Knowledge by domain](using-bcquality.md#knowledge-by-domain) |
|
||||||
|
| Fix setup problems or report an incorrect finding | [Troubleshooting and support](troubleshooting.md) |
|
||||||
|
| Select layers, add company rules, or maintain a fork | [Customizing BCQuality](customizing-bcquality.md) |
|
||||||
|
| Add or improve shared knowledge | [Your first contribution](contributing.md#your-first-contribution) |
|
||||||
|
|
||||||
|
## Integration and technical reference
|
||||||
|
|
||||||
|
These pages are for people building integrations or maintaining skills, not
|
||||||
|
prerequisites for using the plugin.
|
||||||
|
|
||||||
|
| Reference | Purpose |
|
||||||
|
| --- | --- |
|
||||||
|
| [Minimal integration example](agent-consumption.md#try-a-minimal-integration) | A bootstrap prompt connecting your agent to a BCQuality checkout and an app folder. |
|
||||||
|
| [How agents consume BCQuality](agent-consumption.md) | Architecture, repository structure, routing, and delivery of findings. |
|
||||||
|
| [Standalone runner](standalone-runner.md) | Optional model selection, scheduling, retries, and telemetry. |
|
||||||
|
| [Global skills](../skills/README.md) | Host adapters versus internal protocol files. |
|
||||||
|
| [Entry](../skills/entry.md) | Task context and skill dispatch. |
|
||||||
|
| [READ](../skills/read.md) | Knowledge schema, applicability, and precedence. |
|
||||||
|
| [DO](../skills/do.md) | Action-skill format and structured output contract. |
|
||||||
|
| [WRITE](../skills/write.md) | Knowledge-authoring rules. |
|
||||||
|
| [Review evaluation](../evaluation/README.md) | Sample conventions, fixture preparation, and scoring. |
|
||||||
|
|
@ -1,13 +1,78 @@
|
||||||
# How agents consume BCQuality
|
# How agents consume BCQuality
|
||||||
|
|
||||||
BCQuality is content — knowledge files and skills. It is consumed by agents that live elsewhere (AL-Go, a VS Code extension, a GitHub Agent invocation, etc.). This document explains the end-to-end flow, so that skill authors, orchestrator maintainers, and contributors share one mental model.
|
BCQuality is content — knowledge files and skills. It is consumed by agents
|
||||||
|
supplied by a host or orchestrator. This document explains the end-to-end flow
|
||||||
|
so that skill authors, orchestrator maintainers, and contributors share one
|
||||||
|
mental model.
|
||||||
|
|
||||||
For the high-level framing and repo structure, start with the [README](README.md). This document is the operational view.
|
[Documentation](README.md) | [Partner quick start](../README.md#quick-start) | [Runner contract](standalone-runner.md)
|
||||||
|
|
||||||
|
This is the operational reference for integration authors. Partners using
|
||||||
|
the installed plugin do not need to implement this flow themselves.
|
||||||
|
|
||||||
|
## Try a minimal integration
|
||||||
|
|
||||||
|
**"Invoke `skills/entry.md`" means ask your agent to read and follow that
|
||||||
|
instruction document.** It is not a shell command, HTTP endpoint, or executable
|
||||||
|
library. Your host must be able to read files, enumerate directories, and
|
||||||
|
execute the selected skills as instructed. Merely mentioning BCQuality does
|
||||||
|
not make its content available to the model.
|
||||||
|
|
||||||
|
For a first integration, create or reuse a dedicated BCQuality checkout.
|
||||||
|
For example, in PowerShell:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
git clone https://github.com/microsoft/BCQuality.git "C:\Knowledge\BCQuality"
|
||||||
|
```
|
||||||
|
|
||||||
|
Give the host access to **both** that content directory and your own app
|
||||||
|
directory. The plugin is not required for this route. Replace the paths and
|
||||||
|
BC version below with your actual values, then send this prompt to the agent:
|
||||||
|
|
||||||
|
```text
|
||||||
|
BCQuality root: C:\Knowledge\BCQuality
|
||||||
|
Review input: folder-path = C:\Repos\MyBusinessCentralApp
|
||||||
|
|
||||||
|
Read BCQuality's skills\entry.md and follow it with this task context:
|
||||||
|
task-context:
|
||||||
|
goal: Review the complete AL app without changing its source files.
|
||||||
|
inputs-available: [folder-path]
|
||||||
|
technologies: [al]
|
||||||
|
bc-version: 28
|
||||||
|
enabled-layers: [microsoft, community, custom]
|
||||||
|
disabled-skills: []
|
||||||
|
|
||||||
|
Resolve BCQuality instructions, knowledge, and index preparation against the
|
||||||
|
BCQuality root, not the app directory. Pass the actual review-input path above
|
||||||
|
when a dispatched skill accepts folder-path.
|
||||||
|
Follow Entry's preparation and dispatch instructions. Execute every dispatched
|
||||||
|
action skill with its exact input subset, reading READ and DO on demand.
|
||||||
|
Return each complete findings report unchanged. If Entry returns no-match or
|
||||||
|
failed, return that dispatch record unchanged instead of inventing a review.
|
||||||
|
```
|
||||||
|
|
||||||
|
`inputs-available` lists input **types**; the `Review input` line binds the type
|
||||||
|
to the actual app directory. It is not an extra Entry schema field. Omit
|
||||||
|
`bc-version` when unknown rather than guessing it; add localization or
|
||||||
|
application-area context only when known. Keep the two roots distinct so index
|
||||||
|
preparation operates on BCQuality, not your app.
|
||||||
|
|
||||||
|
Expect Entry to select the action skills and the agent to execute them.
|
||||||
|
A broad review normally returns the Microsoft coordinator's report with
|
||||||
|
domain `sub-results`, plus any separately dispatched reports. Each report
|
||||||
|
must retain its outcome, including incomplete or failed work; see
|
||||||
|
[reading results](using-bcquality.md#reading-your-results). A dispatch record
|
||||||
|
alone is not a completed review.
|
||||||
|
|
||||||
|
This prompt delegates the existing protocol rather than implementing new
|
||||||
|
routing logic. For repeatable runs, [pin the checkout](customizing-bcquality.md#updates-and-versions).
|
||||||
|
Add scheduling, retries, and rendering only when needed, using the
|
||||||
|
[runner contract](standalone-runner.md).
|
||||||
|
|
||||||
## The actors
|
## The actors
|
||||||
|
|
||||||
- **Orchestrator** — the tool that triggers work (e.g. AL-Go on a pull request, or a VS Code extension on save). Lives *outside* BCQuality. Knows *when* to run something, not *what* to run.
|
- **Orchestrator** — the tool that triggers work. Lives *outside* BCQuality. Knows *when* to run something, not *what* to run.
|
||||||
- **Agent** — an LLM-driven process spawned by the orchestrator. The agent has no built-in knowledge of BC or of BCQuality's conventions. It knows how to read instructions and call tools.
|
- **Agent** — an LLM-driven process supplied by the host. It brings its own coding knowledge and tools; BCQuality adds curated guidance and execution contracts.
|
||||||
- **BCQuality repo** — two kinds of content:
|
- **BCQuality repo** — two kinds of content:
|
||||||
- **Global skills** in `/skills/` — the `entry.md` entry-point skill plus the READ · DO · WRITE contracts that govern the rest of the repo.
|
- **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.
|
- **Layer content** in `/microsoft/`, `/community/`, and `/custom/` — knowledge files and action skills grouped by authority.
|
||||||
|
|
@ -16,11 +81,30 @@ 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
|
`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.
|
action skill: it creates the task context and enters the same flow at Entry.
|
||||||
|
|
||||||
|
## Repository structure
|
||||||
|
|
||||||
|
| Path | Purpose |
|
||||||
|
| --- | --- |
|
||||||
|
| `skills/entry.md` | Routes a task to action skills. |
|
||||||
|
| `skills/read.md`, `skills/do.md`, `skills/write.md` | Stable knowledge, action-skill, and authoring contracts. |
|
||||||
|
| `skills/al-code-review/SKILL.md` | Host-format plugin adapter. |
|
||||||
|
| `<layer>/knowledge/<domain>/` | Atomic articles and optional sibling samples. |
|
||||||
|
| `<layer>/skills/` | Layer-owned action skills. |
|
||||||
|
| `docs/` | Partner guides and integration references. |
|
||||||
|
| `evaluation/` | Neutral review fixtures and scoring contract. |
|
||||||
|
| `tools/` | Knowledge-index and evaluation tooling. |
|
||||||
|
| `.github/` | Validation and repository workflows. |
|
||||||
|
|
||||||
|
Layers are `microsoft`, `community`, and `custom`; Custom is a template for
|
||||||
|
consumer forks. An action skill either evaluates knowledge directly (a leaf)
|
||||||
|
or composes declared leaves (a super-skill). See [global skills](../skills/README.md)
|
||||||
|
for the distinction between host-native packaging and these internal formats.
|
||||||
|
|
||||||
## The flow
|
## The flow
|
||||||
|
|
||||||
```mermaid
|
```mermaid
|
||||||
flowchart LR
|
flowchart LR
|
||||||
O[Orchestrator<br/>AL-Go] -->|1 trigger + task context| A[Agent]
|
O[Host or orchestrator] -->|1 trigger + task context| A[Agent]
|
||||||
A -->|2 invoke entry.md| E[Entry<br/>routing skill]
|
A -->|2 invoke entry.md| E[Entry<br/>routing skill]
|
||||||
E -->|3 dispatch record| A
|
E -->|3 dispatch record| A
|
||||||
A -->|4 invoke dispatched skill| S[Action skill<br/>e.g. al-code-review]
|
A -->|4 invoke dispatched skill| S[Action skill<br/>e.g. al-code-review]
|
||||||
|
|
@ -66,7 +150,17 @@ At this point the agent reads READ and DO on demand — it needs READ to interpr
|
||||||
|
|
||||||
Discovering candidates at the Source step naively means opening every file under a domain folder just to read its frontmatter `keywords` — on a large corpus that is hundreds of file reads per review. To avoid this, BCQuality maintains a **knowledge index**: a single artifact (`knowledge-index.json`) that lists every article surviving the consumer's layer/allow-deny filtering and carries, per article, the exact inputs the Source/Worklist steps consume — `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint.
|
Discovering candidates at the Source step naively means opening every file under a domain folder just to read its frontmatter `keywords` — on a large corpus that is hundreds of file reads per review. To avoid this, BCQuality maintains a **knowledge index**: a single artifact (`knowledge-index.json`) that lists every article surviving the consumer's layer/allow-deny filtering and carries, per article, the exact inputs the Source/Worklist steps consume — `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint.
|
||||||
|
|
||||||
The index is **owned and produced by BCQuality**, not by each consumer: its generator (`tools/Build-KnowledgeIndex.ps1`) ships here, next to the skills and knowledge it derives from, so the index schema stays in lockstep with the Source contract and every consumer gets the same faithful index for free instead of re-implementing the parser. The consuming orchestrator does **not** build or invoke the index — it only prunes its clone to policy as it already does. The index is then (re)generated by BCQuality itself: **Entry's preparation step runs `Build-KnowledgeIndex.ps1` over the live, already-pruned clone** at the start of every run (see `skills/entry.md`), and BCQuality CI (`.github/workflows/knowledge-index.yml`) validates that the generator is healthy and deterministic. Building over the *pruned* clone — rather than shipping a committed full-corpus index that consumers trust — keeps the index exact for any consumer policy: it can never list an article the consumer denied, so policy-excluded rules cannot leak into discovery.
|
The index is **owned and produced by BCQuality**, not reimplemented by each
|
||||||
|
consumer. Its generator, `tools/Build-KnowledgeIndex.ps1`, ships here alongside
|
||||||
|
the content. Entry ensures the index reflects the live tree before routing
|
||||||
|
and regenerates it when absent or not known to be current. BCQuality CI
|
||||||
|
validates that the generator is healthy and deterministic.
|
||||||
|
|
||||||
|
Consumers with allow/deny policy must prune their content copy **before**
|
||||||
|
Entry runs. Building over that pruned tree prevents removed articles from
|
||||||
|
entering discovery. A standalone plugin normally ships the whole tree:
|
||||||
|
`enabled-layers` filters discovery but does not remove files or enforce a
|
||||||
|
security boundary. See [layer selection](customizing-bcquality.md#select-layers-or-disable-a-review).
|
||||||
|
|
||||||
The index changes only *how candidates are discovered*, never *which are selected*. The Worklist predicate is unchanged — `keywords` still drive selection — and the agent still opens each worklisted article **in full** to read its `## Best Practice` / `## Anti Pattern` rule bodies; the index is discovery metadata only and never substitutes for the article body. When no index is present, skills fall back to path-based discovery (collect by domain folder), so review still works.
|
The index changes only *how candidates are discovered*, never *which are selected*. The Worklist predicate is unchanged — `keywords` still drive selection — and the agent still opens each worklisted article **in full** to read its `## Best Practice` / `## Anti Pattern` rule bodies; the index is discovery metadata only and never substitutes for the article body. When no index is present, skills fall back to path-based discovery (collect by domain folder), so review still works.
|
||||||
|
|
||||||
194
docs/contributing.md
Normal file
194
docs/contributing.md
Normal file
|
|
@ -0,0 +1,194 @@
|
||||||
|
# Contributing to BCQuality
|
||||||
|
|
||||||
|
[Documentation](README.md) | [Knowledge by domain](using-bcquality.md#knowledge-by-domain) | [Authoring reference](../skills/write.md)
|
||||||
|
|
||||||
|
Partners are welcome to contribute shared knowledge, examples, skills, and
|
||||||
|
documentation. To report an incorrect finding without preparing a change,
|
||||||
|
use the [support guide](troubleshooting.md#reporting-a-problem).
|
||||||
|
|
||||||
|
## Your first contribution
|
||||||
|
|
||||||
|
You can propose shared guidance without write access to the upstream
|
||||||
|
repository. Use this path for a correction or a new article:
|
||||||
|
|
||||||
|
1. Search the [existing knowledge](using-bcquality.md#knowledge-by-domain) and
|
||||||
|
[open issues](https://github.com/microsoft/BCQuality/issues). Correct or
|
||||||
|
extend an existing article when it already owns the concern; add a new
|
||||||
|
article only for a distinct concern that meets the admission test below.
|
||||||
|
2. [Fork BCQuality](https://github.com/microsoft/BCQuality/fork) into your GitHub
|
||||||
|
account or organization, clone your fork, and create a working branch from
|
||||||
|
the current upstream `main`. Make edits in that branch, not the plugin cache.
|
||||||
|
3. Choose the [owning layer and domain](#choose-the-right-destination), then
|
||||||
|
edit the article or use the [shared-article starter](#shared-article-starter).
|
||||||
|
A contribution intended for everyone does not belong in `custom/`.
|
||||||
|
4. Add supporting sources and relevant good/bad samples. For a false positive,
|
||||||
|
explain the valid pattern and the mistaken finding the rule should prevent.
|
||||||
|
5. Run the [documented checks](#before-opening-a-pr), then commit and push
|
||||||
|
your branch to your fork.
|
||||||
|
6. On GitHub, open a pull request with **base repository
|
||||||
|
`microsoft/BCQuality`, base branch `main`**, and your fork's working branch
|
||||||
|
as the head. Explain why the change is needed and respond to review by
|
||||||
|
pushing further commits to the same branch.
|
||||||
|
|
||||||
|
Merged content is not automatically loaded into an existing agent session.
|
||||||
|
Consumers must pick up the updated content through their installation or
|
||||||
|
checkout; see [updates and versions](customizing-bcquality.md#updates-and-versions).
|
||||||
|
|
||||||
|
## What belongs here
|
||||||
|
|
||||||
|
BCQuality is a remedial knowledge base. A knowledge file exists because a
|
||||||
|
capable LLM **would get something wrong, or miss something, without it**, not
|
||||||
|
simply because the topic is important. Apply this admission test:
|
||||||
|
|
||||||
|
> If this file did not exist, would a modern LLM reviewing or generating BC
|
||||||
|
> code make a BC-specific mistake that the configured compiler, analyzers, and
|
||||||
|
> tests would not reliably catch, or would it misinterpret or incorrectly
|
||||||
|
> remediate one of their diagnostics?
|
||||||
|
|
||||||
|
Good candidates encode a BC-specific mechanic that models get wrong, a
|
||||||
|
version-dependent behavior, or a misleading interpretation of an analyzer
|
||||||
|
rule. For example:
|
||||||
|
|
||||||
|
- [SetLoadFields and filters can be called in either order](../microsoft/knowledge/performance/use-setloadfields-for-partial-records.md): their relative order does not change the projection. This prevents an incorrect performance finding.
|
||||||
|
- [Boolean page record triggers default to true](../microsoft/knowledge/error-handling/page-boolean-triggers-default-to-true.md): omitting an explicit `exit(true)` is not itself a defect.
|
||||||
|
- [Page fields can inherit captions](../microsoft/knowledge/style/caption-required-on-page-fields.md): an omitted page-level property is not sufficient evidence that a caption is missing.
|
||||||
|
|
||||||
|
Generic advice such as "use HTTPS," "do not hardcode secrets," or "keep
|
||||||
|
transactions short" does not earn a separate knowledge file merely by being
|
||||||
|
sound advice. Negative clarifications that prevent false positives are as
|
||||||
|
valuable as rules that catch defects.
|
||||||
|
|
||||||
|
Do not add knowledge whose anti-pattern is fully and deterministically detected
|
||||||
|
by the AL compiler or a standard analyzer. This applies to authoring as well as
|
||||||
|
review: an authoring agent should compile with the consuming app's actual
|
||||||
|
ruleset and correct the resulting diagnostics instead of carrying prose copies
|
||||||
|
of analyzer rules in context. Analyzer-related knowledge belongs here only when
|
||||||
|
it adds a BC-specific exception, version boundary, cross-object implication, or
|
||||||
|
remediation constraint that the diagnostic itself cannot establish. Merely
|
||||||
|
explaining why a deterministic rule exists is not sufficient.
|
||||||
|
|
||||||
|
**Skills hold discovery and execution mechanics; knowledge files hold BC
|
||||||
|
facts.** Correct or extend a knowledge article when a BC fact is missing or
|
||||||
|
wrong. Do not hide that fact in a skill's instructions. A genuine routing,
|
||||||
|
input, or output-contract problem belongs in the skill instead.
|
||||||
|
|
||||||
|
## Choose the right destination
|
||||||
|
|
||||||
|
| Change | Destination |
|
||||||
|
| --- | --- |
|
||||||
|
| Knowledge in a Microsoft-owned review domain | `microsoft/knowledge/<domain>/` |
|
||||||
|
| Knowledge accompanying a Community-owned skill | `community/knowledge/<domain>/` |
|
||||||
|
| Company-specific policy or an override | `custom/` in your own fork; never an upstream contribution |
|
||||||
|
| Partner instructions or how-to guidance | `docs/`, linked from the documentation index |
|
||||||
|
|
||||||
|
Layer ownership follows the skill and domain, **not your employer**. For
|
||||||
|
example, a partner's performance clarification belongs beside the Microsoft
|
||||||
|
performance skill's corpus. Do not use Community as a staging area for an
|
||||||
|
already Microsoft-owned domain. A split may exist briefly during promotion,
|
||||||
|
but the skill and its canonical corpus should move together.
|
||||||
|
|
||||||
|
Upstream automatically closes PRs adding custom content. Follow
|
||||||
|
[Customizing BCQuality](customizing-bcquality.md) for organization-only rules.
|
||||||
|
Do not introduce a new shared domain without the action skill that consumes
|
||||||
|
it and the matching evaluation samples.
|
||||||
|
|
||||||
|
## Author a knowledge article
|
||||||
|
|
||||||
|
Read [READ](../skills/read.md) for the schema and
|
||||||
|
[WRITE](../skills/write.md) for the authoring rules. Use an existing article
|
||||||
|
in the same domain as a starting point, then remove unrelated guidance.
|
||||||
|
|
||||||
|
Every article has six required frontmatter fields: `bc-version`, `domain`,
|
||||||
|
`keywords`, `technologies`, `countries`, and `application-area`.
|
||||||
|
`domain` must match its containing directory. Keep one concern per file,
|
||||||
|
ideally under 50 lines and no more than 100.
|
||||||
|
|
||||||
|
`Description` is required. Put recommendations in `Best Practice` and mistakes
|
||||||
|
to catch in `Anti Pattern`; those are the normative sections. Explain
|
||||||
|
legitimate exceptions so a reviewer does not turn a useful rule into a false
|
||||||
|
positive. Code fences are not allowed in knowledge articles.
|
||||||
|
|
||||||
|
### Shared-article starter
|
||||||
|
|
||||||
|
Use [caption-required-on-page-fields.md](../microsoft/knowledge/style/caption-required-on-page-fields.md)
|
||||||
|
as a complete shared-knowledge example. It demonstrates all six metadata
|
||||||
|
fields, a clear concern, normative guidance and exceptions, linked good/bad
|
||||||
|
samples, and authoritative sources.
|
||||||
|
|
||||||
|
For a new concern, follow that structure but choose your own descriptive
|
||||||
|
filename, domain, applicability, keywords, and guidance. Replace its sources
|
||||||
|
and sample links with ones supporting your concern; do not duplicate the
|
||||||
|
caption rule. If you are correcting caption guidance itself, edit the
|
||||||
|
existing article instead. Use a company-only rule only in your fork's Custom
|
||||||
|
layer, following the separate [customization example](customizing-bcquality.md#add-an-organization-specific-rule).
|
||||||
|
|
||||||
|
### Sources and examples
|
||||||
|
|
||||||
|
When adding or changing a platform claim, link the authoritative source that
|
||||||
|
supports it, preferably the specific Microsoft Learn API/property page or a
|
||||||
|
public source definition. State version constraints when they matter. Avoid
|
||||||
|
"upstream guidance says" without a link. If the source is unavailable or the
|
||||||
|
guidance is organization policy or empirical observation, say so explicitly
|
||||||
|
rather than presenting it as an official platform guarantee.
|
||||||
|
|
||||||
|
Place source links in a short `References` section or beside the relevant
|
||||||
|
claim. References do not replace the rule: keep all load-bearing guidance in
|
||||||
|
the normative sections. This adds traceability without adding frontmatter
|
||||||
|
fields or changing the schema.
|
||||||
|
|
||||||
|
Put demonstration code in sibling files:
|
||||||
|
|
||||||
|
```text
|
||||||
|
<slug>.md
|
||||||
|
<slug>.good.al
|
||||||
|
<slug>.bad.al
|
||||||
|
```
|
||||||
|
|
||||||
|
Reference each sample with a clickable link whose label retains the filename,
|
||||||
|
for example `` [`<slug>.good.al`](<slug>.good.al) `` with your actual slug.
|
||||||
|
One or both samples are optional for an individual article; every review
|
||||||
|
domain must have at least one complete good/bad pair for evaluation. Samples
|
||||||
|
are self-contained demonstrations, not copied Base Application source and
|
||||||
|
not a deployable or compiled application.
|
||||||
|
|
||||||
|
## Before opening a PR
|
||||||
|
|
||||||
|
From your BCQuality checkout, use the existing validators. The Python
|
||||||
|
validator needs Python and PyYAML; the fixture harness needs PowerShell 7.5 or later
|
||||||
|
so findings-report parsing preserves timestamp-shaped JSON strings verbatim.
|
||||||
|
If PyYAML is not installed in your development environment, install it with
|
||||||
|
`python -m pip install pyyaml`.
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
python .github\scripts\validate_frontmatter.py --root .
|
||||||
|
pwsh .\tools\Test-ReviewFixtures.ps1 -Root .
|
||||||
|
pwsh .\tools\Test-ReviewContract.ps1 -Root .
|
||||||
|
```
|
||||||
|
|
||||||
|
The first command checks schema, sections, naming, sample references, and
|
||||||
|
skill registration. The second checks that every review leaf has a valid
|
||||||
|
positive/clean sample pair. The third checks the cross-surface findings-report
|
||||||
|
contract and its bounded range-normalization cases. None proves a model will
|
||||||
|
find every defect. See [evaluation](../evaluation/README.md) for optional
|
||||||
|
model-based scoring.
|
||||||
|
|
||||||
|
In the PR description, explain the mistake being prevented, supporting
|
||||||
|
evidence, applicable BC versions, and why the chosen domain owns it. For a
|
||||||
|
false positive, include the valid pattern and the incorrect finding being
|
||||||
|
prevented. Check that links and samples open from the rendered article.
|
||||||
|
|
||||||
|
Schema and stable protocol changes require approval from both maintainers.
|
||||||
|
Avoid repeating schema or contract definitions in new guides: link the
|
||||||
|
canonical READ, DO, WRITE, or Entry section instead.
|
||||||
|
|
||||||
|
## Content releases
|
||||||
|
|
||||||
|
Maintainers cut content releases on demand, roughly monthly, using the
|
||||||
|
`Release version` workflow on `main`. It tags the selected commit as
|
||||||
|
`v{major}.{minor}`; it does not update the plugin manifest.
|
||||||
|
|
||||||
|
Use a minor bump for normal content updates and a major bump for breaking
|
||||||
|
changes. The minor is a monotonic counter: it increments across releases and
|
||||||
|
does **not** reset on a major bump. See
|
||||||
|
[updates and versions](customizing-bcquality.md#updates-and-versions) for the
|
||||||
|
separate plugin, content, and skill version identifiers.
|
||||||
182
docs/customizing-bcquality.md
Normal file
182
docs/customizing-bcquality.md
Normal file
|
|
@ -0,0 +1,182 @@
|
||||||
|
# Customizing BCQuality
|
||||||
|
|
||||||
|
[Documentation](README.md) | [Using BCQuality](using-bcquality.md) | [Contributing](contributing.md)
|
||||||
|
|
||||||
|
**No customization is required to get started.** Use the upstream plugin
|
||||||
|
unless you need a different review selection or organization-specific rules.
|
||||||
|
Model choice, concurrency, retries, and billing belong to your host, not
|
||||||
|
BCQuality. A [standalone runner](standalone-runner.md) is an advanced option.
|
||||||
|
|
||||||
|
## Select layers or disable a review
|
||||||
|
|
||||||
|
The standalone adapter reads these environment variables from the process
|
||||||
|
that starts your host:
|
||||||
|
|
||||||
|
| Variable | Default | Meaning |
|
||||||
|
| --- | --- | --- |
|
||||||
|
| `BCQUALITY_ENABLED_LAYERS` | `microsoft,community,custom` | Comma-separated layer names to discover. |
|
||||||
|
| `BCQUALITY_DISABLED_SKILLS` | None | Comma-separated **BCQuality repo-relative skill paths** to exclude, not display names or knowledge-article paths. |
|
||||||
|
|
||||||
|
For example, in PowerShell, enable only Microsoft knowledge and omit the
|
||||||
|
dedicated style review:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
$env:BCQUALITY_ENABLED_LAYERS = "microsoft"
|
||||||
|
$env:BCQUALITY_DISABLED_SKILLS = "microsoft/skills/review/al-style-review.md"
|
||||||
|
copilot
|
||||||
|
```
|
||||||
|
|
||||||
|
Set the variables **before** starting a new session. They apply to that
|
||||||
|
terminal and its child processes; use your host's environment configuration
|
||||||
|
if it starts elsewhere. Review selection is not a guarantee that another
|
||||||
|
domain or the agent will never mention a related concern.
|
||||||
|
|
||||||
|
To return to defaults, remove those variables from the environment before
|
||||||
|
starting the host again (or use a fresh terminal if you only set them there).
|
||||||
|
Do not use an empty comma-separated value as a substitute for the default.
|
||||||
|
|
||||||
|
All layers are enabled by default. Where relevant articles have overlapping
|
||||||
|
applicability and **contradictory guidance**, precedence is:
|
||||||
|
|
||||||
|
**Custom > Community > Microsoft.**
|
||||||
|
|
||||||
|
Otherwise the layers are additive. A matching filename alone does not suppress
|
||||||
|
an article; the [READ contract](../skills/read.md#layer-precedence) governs
|
||||||
|
knowledge conflicts. Review reports record displaced knowledge in `suppressed`.
|
||||||
|
|
||||||
|
Action skills use the same layer order but override by frontmatter `id`. A
|
||||||
|
custom leaf with the same `id` as a Community or Microsoft leaf replaces that
|
||||||
|
leaf in every super-skill slot while its layer is enabled. The files may have
|
||||||
|
different names. IDs must remain unique within each layer. Disabling the custom
|
||||||
|
layer or the custom skill path makes composition fall back to the next enabled
|
||||||
|
implementation. Hosts should build the skill index and use
|
||||||
|
`tools/Resolve-SkillWorklist.ps1`; they must not implement this selection from
|
||||||
|
filenames.
|
||||||
|
|
||||||
|
Layer selection is **not an access-control boundary**. A plugin installation
|
||||||
|
still contains excluded layers on disk. An integration requiring genuine
|
||||||
|
exclusion must remove denied files from its own content copy before the agent
|
||||||
|
reads it; the [adapter](../skills/al-code-review/SKILL.md#layer-selection-is-not-a-deny-mechanism)
|
||||||
|
explains this distinction.
|
||||||
|
|
||||||
|
## Add an organization-specific rule
|
||||||
|
|
||||||
|
Keep custom content in a fork or organization-controlled copy of BCQuality,
|
||||||
|
not in your AL app's `custom` folder and not in the installed plugin cache.
|
||||||
|
Editing the cache is not durable across updates.
|
||||||
|
|
||||||
|
1. Fork BCQuality into a repository your organization controls, or create an
|
||||||
|
organization-controlled copy if a public fork is unsuitable for your policy.
|
||||||
|
2. Clone that repository and run `git remote get-url origin`. Confirm it is
|
||||||
|
your repository, **not** `microsoft/BCQuality`.
|
||||||
|
3. Add the article under `custom/knowledge/<existing-domain>/`, using the
|
||||||
|
[knowledge format](../skills/read.md). Keep your company's content out of
|
||||||
|
upstream pull requests.
|
||||||
|
|
||||||
|
For example, suppose your company deliberately names one page "ACME Inventory
|
||||||
|
Workbench" while showing stockkeeping units, and already makes the row type
|
||||||
|
clear in its UI. You want a narrow exception to the shared page-naming rule.
|
||||||
|
Create `custom/knowledge/style/page-name-must-match-source-table.md` in your
|
||||||
|
copy with this content:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: style
|
||||||
|
keywords: [page-name, source-table, inventory, workbench]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Allow the ACME Inventory Workbench task name
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Our approved page "ACME Inventory Workbench" shows stockkeeping units. Its UI
|
||||||
|
identifies the row type explicitly; its task-oriented name is company policy.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Do not report that page solely because its name differs from its source-table
|
||||||
|
entity. Keep the shared naming guidance for other pages.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Renaming the approved page solely to repeat the source-table entity, or
|
||||||
|
applying this exception to an unrelated page.
|
||||||
|
```
|
||||||
|
|
||||||
|
This is an **illustrative company policy**, not a new Microsoft recommendation.
|
||||||
|
Choose your actual domain, applicability, and policy; do not broaden an
|
||||||
|
exception merely to silence a valid defect. The shared rule is
|
||||||
|
[page-name-must-match-source-table.md](../microsoft/knowledge/style/page-name-must-match-source-table.md).
|
||||||
|
Guidance in `Best Practice` and `Anti Pattern` drives conflict resolution, so
|
||||||
|
do not put the exception only in a non-normative notes section.
|
||||||
|
|
||||||
|
Follow the [contribution checks](contributing.md#before-opening-a-pr) locally,
|
||||||
|
then commit your change in your repository. A new knowledge domain also needs
|
||||||
|
an action skill that discovers it; adding an arbitrary folder does not create
|
||||||
|
a review.
|
||||||
|
|
||||||
|
## Use your fork
|
||||||
|
|
||||||
|
Adding custom content does not change the upstream plugin you already
|
||||||
|
installed. Point the host at your copy.
|
||||||
|
|
||||||
|
For a pushed fork, replace `YOUR-ORG` with its owner. These commands replace
|
||||||
|
the upstream installation, since both manifests use the name `bcquality`:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
copilot plugin uninstall bcquality
|
||||||
|
copilot plugin install YOUR-ORG/BCQuality
|
||||||
|
copilot plugin list
|
||||||
|
```
|
||||||
|
|
||||||
|
For local development, install your copy's absolute path instead:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
copilot plugin install "C:\Repos\CompanyBCQuality"
|
||||||
|
```
|
||||||
|
|
||||||
|
Direct local installs are cached by the CLI; reinstall that path after edits,
|
||||||
|
then start a new session. See the host's
|
||||||
|
[local-plugin instructions](https://docs.github.com/en/copilot/how-tos/copilot-cli/customize-copilot/plugins-creating).
|
||||||
|
Do not assume the currently running session has reloaded the content.
|
||||||
|
|
||||||
|
Confirm the plugin list points at the intended source and that the Custom
|
||||||
|
layer is enabled. Review a small example relevant to your rule. Ask the host
|
||||||
|
which custom article it read and inspect `suppressed` for an actual conflict.
|
||||||
|
The negative-rule example should produce no naming finding for the approved
|
||||||
|
page; do not add an information-only finding just to prove the article was
|
||||||
|
loaded. The absence of a finding alone does not prove your fork was used.
|
||||||
|
|
||||||
|
An external runner should likewise read from your fork or local copy rather
|
||||||
|
than the upstream URL. It must still start at [Entry](../skills/entry.md).
|
||||||
|
|
||||||
|
## Updates and versions
|
||||||
|
|
||||||
|
| Identifier | What it identifies |
|
||||||
|
| --- | --- |
|
||||||
|
| Plugin `version` in `plugin.json` | The host-facing package version. It is separate from content-release tags. `copilot plugin list` shows the installed plugin; inspect the resolved source when reproducing a run. |
|
||||||
|
| Content tag such as `v1.6` | A release of the repository's knowledge and skills. Available tags are listed on [GitHub](https://github.com/microsoft/BCQuality/tags). |
|
||||||
|
| Skill `version` in frontmatter | That skill's contract version, carried in reports. It does not identify the complete knowledge snapshot. |
|
||||||
|
| Git commit SHA | The exact repository snapshot. Record this for reproducibility when using a checkout. |
|
||||||
|
|
||||||
|
For the upstream plugin, run `copilot plugin update bcquality`, then start a
|
||||||
|
new session. The unpinned installation command does not promise a particular
|
||||||
|
content-release tag. For a fork, updating the plugin reads your fork; it does
|
||||||
|
not merge upstream changes into it.
|
||||||
|
|
||||||
|
To maintain a fork, commit your custom work first, add an `upstream` remote
|
||||||
|
pointing to `https://github.com/microsoft/BCQuality.git` once, fetch upstream,
|
||||||
|
and merge the desired upstream branch or content tag. Resolve conflicts and
|
||||||
|
review the resulting policy before publishing or reinstalling your fork.
|
||||||
|
Do not overwrite the fork wholesale with an upstream download.
|
||||||
|
|
||||||
|
For repeatable CI or runner use, select a tag or commit in a dedicated clean
|
||||||
|
checkout and record `git rev-parse HEAD`. Upgrade deliberately, compare the
|
||||||
|
old and new content, and rerun representative reviews. To roll back, select
|
||||||
|
the previously recorded snapshot in that checkout and reinstall it if your
|
||||||
|
host caches local plugins. Retain organization-specific rules in the chosen
|
||||||
|
snapshot rather than reverting to an upstream-only tag.
|
||||||
188
docs/standalone-runner.md
Normal file
188
docs/standalone-runner.md
Normal file
|
|
@ -0,0 +1,188 @@
|
||||||
|
# Build a lightweight standalone review runner
|
||||||
|
|
||||||
|
[Documentation](README.md) | [Architecture](agent-consumption.md)
|
||||||
|
|
||||||
|
BCQuality provides review knowledge, routing, execution instructions, and
|
||||||
|
structured output contracts. It intentionally does not choose models, schedule
|
||||||
|
agents, retry failures, or collect usage telemetry. A standalone runner can add
|
||||||
|
those host-specific capabilities without copying Business Central rules out of
|
||||||
|
BCQuality.
|
||||||
|
|
||||||
|
Use the built-in standalone plugin when the host's default execution is
|
||||||
|
sufficient. Build a runner when you need explicit control over cost, latency,
|
||||||
|
concurrency, or integration with another review surface.
|
||||||
|
|
||||||
|
Start with the [minimal integration example](agent-consumption.md#try-a-minimal-integration)
|
||||||
|
to connect your agent to the content before adding runner-specific behavior.
|
||||||
|
|
||||||
|
## Keep BCQuality current
|
||||||
|
|
||||||
|
For plugin installation, use the [quick start](../README.md#quick-start).
|
||||||
|
For version identifiers, forks, and reproducible snapshots, see
|
||||||
|
[updates and versions](customizing-bcquality.md#updates-and-versions).
|
||||||
|
|
||||||
|
A runner that reads BCQuality from a checkout should pin a commit or release
|
||||||
|
and upgrade it deliberately. Do not copy knowledge files or action-skill prose
|
||||||
|
into the runner; doing so creates a second, drifting quality policy.
|
||||||
|
|
||||||
|
## Review a complete app folder
|
||||||
|
|
||||||
|
For a committed app, generated fixture, or source tree that has no meaningful
|
||||||
|
diff, supply the app's root directory as `folder-path`. The review scope is
|
||||||
|
every relevant file below that directory, including `app.json` and AL source.
|
||||||
|
The folder does not need to be a Git repository.
|
||||||
|
|
||||||
|
The [app-review example](../README.md#example-review-a-complete-app-folder)
|
||||||
|
uses this input through the standalone adapter. Because a folder is a
|
||||||
|
current-state snapshot, the review must not invent a previous app version
|
||||||
|
when evaluating comparison-only rules. Entry can return more than one
|
||||||
|
top-level skill; preserve all reports, including separately dispatched
|
||||||
|
Community reviews, rather than assuming the Microsoft coordinator is the
|
||||||
|
only result.
|
||||||
|
|
||||||
|
## Minimal runner flow
|
||||||
|
|
||||||
|
1. Give the agent the review input and a task context containing the user's
|
||||||
|
actual goal, available input types, and any known BC applicability
|
||||||
|
dimensions.
|
||||||
|
2. Invoke `skills/entry.md`. Entry prepares the knowledge index and returns the
|
||||||
|
action skills to run. Do not reproduce its routing logic.
|
||||||
|
3. Execute every dispatched action skill with the exact input subset in its
|
||||||
|
dispatch record. Read `skills/read.md` and `skills/do.md` on demand.
|
||||||
|
4. When an action skill declares `sub-skills`, resolve its ordered leaf slots
|
||||||
|
with `tools/Resolve-SkillWorklist.ps1`, passing the enabled layers and
|
||||||
|
disabled skill paths from the task context. Execute every resolved leaf as
|
||||||
|
a discrete invocation. Leaves are independent and may be scheduled serially
|
||||||
|
or concurrently. Before dispatch, preserve the final ordered selection and
|
||||||
|
legitimate configuration/input-compatibility exclusions as a private expected
|
||||||
|
composition artifact; see [composition acceptance](#composition-acceptance).
|
||||||
|
5. Capture the exact Task return as the immutable raw audit payload and primary
|
||||||
|
transport. Preserve it unchanged in private artifacts or host logs. Before
|
||||||
|
the full DO acceptance gate, create a normalized candidate only for DO's
|
||||||
|
bounded optional-range case, record that normalization separately in private
|
||||||
|
telemetry, and accept the candidate only if the entire copy passes the
|
||||||
|
unchanged strict gate. Use `tools/Validate-FindingsReport.ps1`, passing the
|
||||||
|
exact source paths and fully retrieved article paths; pass `-SkillKind super`
|
||||||
|
and `-ExpectedCompositionPath` for the final rolled-up report. The accepted report contains no undeclared
|
||||||
|
telemetry fields.
|
||||||
|
6. Collect each accepted findings-report into `sub-results` in the declared
|
||||||
|
`sub-skills` order, not completion order. Run the super-skill self-review
|
||||||
|
only after all leaves have finished.
|
||||||
|
7. Apply the DO composition, failure, deduplication, reference-integrity, and
|
||||||
|
outcome rules. Return strict JSON before rendering it for people or another
|
||||||
|
system.
|
||||||
|
|
||||||
|
The runner must never inspect the diff to skip a review domain. A leaf decides
|
||||||
|
its own task-level applicability and reports `not-applicable` or
|
||||||
|
`no-knowledge`.
|
||||||
|
|
||||||
|
## Composition acceptance
|
||||||
|
|
||||||
|
The findings-report validator requires PowerShell 7.5 or later to preserve
|
||||||
|
literal JSON strings with `ConvertFrom-Json -DateKind String`.
|
||||||
|
|
||||||
|
A report can be internally consistent while omitting a selected review. Bind
|
||||||
|
the final acceptance gate to the host's selection, not just the returned
|
||||||
|
reports. The private JSON input to `-ExpectedCompositionPath` follows the
|
||||||
|
[DO consumer acceptance contract](../skills/do.md#consumer-acceptance-gate).
|
||||||
|
For example, if style is selected and security was disabled:
|
||||||
|
|
||||||
|
```json
|
||||||
|
{
|
||||||
|
"superSkill": { "id": "al-code-review", "version": 1 },
|
||||||
|
"subSkills": [{ "id": "al-style-review", "version": 1 }],
|
||||||
|
"skipped": [{ "id": "al-security-review", "version": 1, "reason": "configuration" }],
|
||||||
|
"acceptedResults": []
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Build this artifact from `Resolve-SkillWorklist.ps1` and the input-compatibility
|
||||||
|
decision before dispatch. Resolver `skipped` entries have no version: enrich
|
||||||
|
them from the declared skill's indexed version. Move an input-incompatible
|
||||||
|
selected slot to `skipped` with its selected version and `not-applicable`
|
||||||
|
reason; never use source content or later model output to make that decision.
|
||||||
|
Keep paths, layers, and other resolver metadata if useful for private audit.
|
||||||
|
Do not add the artifact to the findings-report or overwrite it to hide an
|
||||||
|
unfinished invocation. Preserve it with the run's raw payloads.
|
||||||
|
|
||||||
|
Initialize `acceptedResults` before dispatch. After accepting a leaf, save its
|
||||||
|
exact accepted copy (including any permitted normalization) in an immutable
|
||||||
|
host-owned file outside worker/composer write access. Append only its capture
|
||||||
|
to `acceptedResults`, for example:
|
||||||
|
|
||||||
|
```json
|
||||||
|
{"id": "al-style-review", "version": 1, "reportPath": "accepted/style.json"}
|
||||||
|
```
|
||||||
|
|
||||||
|
`reportPath` may be absolute or relative to the composition artifact's directory.
|
||||||
|
Capture a host-created failed validation report the same way. Never construct
|
||||||
|
these captures from the composed `sub-results` or permit the composing model
|
||||||
|
to supply or alter them. Keep the original selection and exclusions unchanged.
|
||||||
|
|
||||||
|
The gate rejects repeated or unexpected leaf IDs, wrong selected versions,
|
||||||
|
reordered results, fabricated or missing exclusions, and uncaptured or altered
|
||||||
|
leaf content. JSON property order is immaterial; array order, field presence,
|
||||||
|
types, and values must match. Every captured leaf must appear in `sub-results`.
|
||||||
|
If selected leaves remain unfinished, the report must be `partial` with a non-failed returned
|
||||||
|
report, otherwise `failed`; name every unfinished leaf ID exactly in
|
||||||
|
`outcome-reason`. Top-level `from-sub-skill: "agent"` findings are rejected
|
||||||
|
while selected leaf results are missing.
|
||||||
|
Do not fabricate leaf reports or use configuration skips for budget exhaustion.
|
||||||
|
Wait for started invocations to finish; omit the self-review if the selected
|
||||||
|
composition remains incomplete. Coverage still sums the non-failed leaf
|
||||||
|
knowledge worklists, not the number of selected leaf slots.
|
||||||
|
|
||||||
|
Calls without the expected artifact remain supported for structural/semantic
|
||||||
|
validation, but cannot certify that a composed review covered its selection.
|
||||||
|
They also cannot bind nested leaves to the host's accepted outputs.
|
||||||
|
They still reject duplicate leaf IDs and returned-and-skipped conflicts.
|
||||||
|
|
||||||
|
## Runner-owned choices
|
||||||
|
|
||||||
|
Keep these settings and behaviors outside BCQuality:
|
||||||
|
|
||||||
|
- coordinator and leaf models;
|
||||||
|
- serial or concurrent scheduling and maximum concurrency;
|
||||||
|
- retries, timeouts, and rate-limit handling;
|
||||||
|
- token, cost, duration, and actual-concurrency telemetry;
|
||||||
|
- conversion of the findings report into Markdown, annotations, or PR
|
||||||
|
comments.
|
||||||
|
|
||||||
|
Model selection and requested concurrency are deployment choices, not review
|
||||||
|
rules. Evaluate them against representative applications before making them a
|
||||||
|
default. Report actual usage and concurrency only when the host exposes native
|
||||||
|
evidence; do not infer them from the requested profile.
|
||||||
|
|
||||||
|
## Failure and output checklist
|
||||||
|
|
||||||
|
A compatible runner:
|
||||||
|
|
||||||
|
- invokes every worklisted leaf exactly once unless a documented retry replaces
|
||||||
|
a failed attempt;
|
||||||
|
- resolves same-ID leaf implementations by `custom > community > microsoft`,
|
||||||
|
preserves declared slot order, and falls back when a higher layer is disabled;
|
||||||
|
- keeps leaf contexts isolated and passes only the inputs they declare;
|
||||||
|
- preserves each raw Task return unchanged for audit and distinguishes it from
|
||||||
|
any normalized accepted copy;
|
||||||
|
- removes only an optional range whose positive integer bounds contain the
|
||||||
|
primary line but start before it, and only when the complete report has no
|
||||||
|
other defect and the finding has no `suggested-code`;
|
||||||
|
- records normalization only in private runner telemetry and never adds fields
|
||||||
|
to the findings-report;
|
||||||
|
- rejects reversed, invalid, or out-of-bounds ranges, range mismatches attached
|
||||||
|
to `suggested-code`, and every repair outside DO's bounded exception;
|
||||||
|
- preserves every leaf report, including failed reports, in `sub-results`;
|
||||||
|
- excludes unreliable findings from failed leaves and returns `partial` when
|
||||||
|
only part of the review is reliable;
|
||||||
|
- orders `sub-results` by the declared worklist and orders rendered findings
|
||||||
|
deterministically;
|
||||||
|
- validates composition against the host-owned selection, and reports selected
|
||||||
|
leaves left unfinished as incomplete rather than clean or configured away;
|
||||||
|
- calculates top-level severity counts from deduplicated top-level findings,
|
||||||
|
not by summing leaf counts;
|
||||||
|
- preserves knowledge paths verbatim and verifies references before publishing;
|
||||||
|
- records the BCQuality commit or release used for the run.
|
||||||
|
|
||||||
|
A CI integration, custom agent, or small host-native plugin can implement this
|
||||||
|
runner contract. These remain optional consumers: BCQuality's knowledge and
|
||||||
|
skills stay independent of their orchestration choices.
|
||||||
59
docs/troubleshooting.md
Normal file
59
docs/troubleshooting.md
Normal file
|
|
@ -0,0 +1,59 @@
|
||||||
|
# Troubleshooting and support
|
||||||
|
|
||||||
|
[Documentation](README.md) | [Quick start](../README.md#quick-start) | [Using BCQuality](using-bcquality.md)
|
||||||
|
|
||||||
|
## Setup and skill discovery
|
||||||
|
|
||||||
|
Run terminal commands outside the interactive Copilot prompt unless they
|
||||||
|
start with `/`.
|
||||||
|
|
||||||
|
| Symptom | What to do |
|
||||||
|
| --- | --- |
|
||||||
|
| `copilot` is not recognized | [Install Copilot CLI](https://docs.github.com/en/copilot/get-started/cli-quickstart), then open a new terminal. Installing Copilot Chat in an editor is not the same step. |
|
||||||
|
| The CLI has no `plugin` command | Update Copilot CLI using its installation method. Confirm `copilot plugin --help` works. |
|
||||||
|
| Sign-in, entitlement, or organization-policy error | Start `copilot`, use `/login`, and confirm your account is allowed to use Copilot CLI. Ask your administrator about organization restrictions; BCQuality cannot override them. |
|
||||||
|
| Plugin installation cannot reach the repository | Confirm access to `https://github.com/microsoft/BCQuality` and follow your organization's proxy/network guidance. Do not disable certificate checks. |
|
||||||
|
| The plugin installed, but the skill is missing | Run `copilot plugin list` in the terminal. Enable it with `copilot plugin enable bcquality` if disabled, then start a new session. In the session, use `/skills list` and look for `al-code-review`. |
|
||||||
|
| The agent performs a generic review | Name the **installed `al-code-review` skill** explicitly, as in the quick start. Ask which skill and BCQuality source it used. Another plugin or host may expose a similarly named operation. |
|
||||||
|
| Installation works in the terminal, but not in the editor | Plugin discovery is host-specific. Follow the editor's installation instructions; a CLI installation is not proof that another host loaded the plugin. |
|
||||||
|
| `pwsh` is missing, or index generation fails | The index is an accelerator, not required knowledge. The review can fall back to discovery from folders. For faster discovery, install [PowerShell 7](https://learn.microsoft.com/en-us/powershell/scripting/install/installing-powershell), or resolve the reported filesystem error. |
|
||||||
|
| An update or local edit is not visible | Run `copilot plugin update bcquality` for a repository-installed plugin, then start a fresh session. For a directly installed local folder, reinstall that folder to refresh the cached copy; see [customizing](customizing-bcquality.md#use-your-fork). |
|
||||||
|
|
||||||
|
## Review results
|
||||||
|
|
||||||
|
| Symptom | What to do |
|
||||||
|
| --- | --- |
|
||||||
|
| `partial`, a timeout, or an unfinished review | Read `outcome-reason` and domain reports. Retry the incomplete scope in a fresh session, use smaller app folders or a focused review, or select a host/model with sufficient capacity. Keep the limited scope visible; do not relabel it a complete app review. |
|
||||||
|
| `failed` | Resolve the stated problem, such as inaccessible input, a failed invocation, or an unverifiable reference, before using that report. A failed domain's findings are not reliable. |
|
||||||
|
| `no-match` or `not-applicable` | Confirm you supplied AL source, the intended folder/file/diff, and an appropriate goal. Check [disabled skills and layers](customizing-bcquality.md#select-layers-or-disable-a-review). |
|
||||||
|
| `no-knowledge` | Check the target BC version, selected domain, enabled layers, and whether the relevant knowledge files are present. No applicable rules is different from no defects. |
|
||||||
|
| `completed` with no findings | This can be a valid clean result for the selected scope. Confirm the intended files and domain reports are included. If you have a concrete missed defect, report it with a minimal example. |
|
||||||
|
| JSON rather than a readable summary | JSON is the shared output format. Ask the host to summarize the existing reports, preserving outcomes, locations, severity, confidence, and references. |
|
||||||
|
| A surprising finding | Open its guidance and samples, inspect surrounding code, and confirm version/localization assumptions. Ask the agent to explain the evidence; do not apply a suggestion solely because it has high confidence. |
|
||||||
|
| The Agents domain is absent | Agents is a separate Community review, not a child of the Microsoft broad review. Explicitly request an Agent SDK review and confirm the Community layer is enabled. |
|
||||||
|
| A missing base branch or unavailable source definition | Supply the real baseline or dependency definition. Without it, do not accept claims that rely on invented history or assumed dependency behavior. |
|
||||||
|
| Slow or expensive review | A broad review makes separate passes over multiple domains. Verify index generation succeeded, use a focused task when appropriate, and inspect usage in your host. BCQuality does not choose models, promise runtimes, or meter charges. |
|
||||||
|
|
||||||
|
## Reporting a problem
|
||||||
|
|
||||||
|
For incorrect BC guidance, missed findings, documentation gaps, or skill
|
||||||
|
behavior, [search existing issues](https://github.com/microsoft/BCQuality/issues)
|
||||||
|
and [open a BCQuality issue](https://github.com/microsoft/BCQuality/issues/new/choose)
|
||||||
|
if needed. You do not have to author a knowledge file before asking for help.
|
||||||
|
Host installation, authentication, billing, or policy problems belong with the
|
||||||
|
host's support channel or your organization administrator.
|
||||||
|
|
||||||
|
Include:
|
||||||
|
|
||||||
|
- The host and version, selected model if known, and BCQuality source/version
|
||||||
|
or commit. See [version identifiers](customizing-bcquality.md#updates-and-versions).
|
||||||
|
- The prompt, scope (folder/file/diff and comparison base), target BC version,
|
||||||
|
and relevant layer/skill settings.
|
||||||
|
- Expected versus actual behavior, the outcome/reason, and the exact rule
|
||||||
|
reference for a disputed finding.
|
||||||
|
- A **minimal, sanitized** AL example or report excerpt that reproduces the
|
||||||
|
problem. Remove secrets, customer data, and proprietary content you cannot share.
|
||||||
|
|
||||||
|
For security vulnerabilities, follow [SECURITY.md](../SECURITY.md) instead of
|
||||||
|
opening a public issue. To contribute a correction yourself, follow the
|
||||||
|
[contribution guide](contributing.md).
|
||||||
269
docs/using-bcquality.md
Normal file
269
docs/using-bcquality.md
Normal file
|
|
@ -0,0 +1,269 @@
|
||||||
|
# Using BCQuality
|
||||||
|
|
||||||
|
[Documentation](README.md) | [Quick start](../README.md#quick-start) | [Troubleshooting](troubleshooting.md)
|
||||||
|
|
||||||
|
BCQuality is knowledge you can read and reuse, plus skills that tell an agent
|
||||||
|
how to apply it. You do not need an AI tool to read the articles. When using
|
||||||
|
an agent, your host supplies authentication, model access, tools, permissions,
|
||||||
|
and rendering; BCQuality does not install a BC extension or an agent.
|
||||||
|
|
||||||
|
## Choose how to use BCQuality
|
||||||
|
|
||||||
|
| Path | What to do |
|
||||||
|
| --- | --- |
|
||||||
|
| Read the knowledge yourself | Browse [knowledge by domain](#knowledge-by-domain), or search the repository for an AL concept. Read the article and its samples. No installation required. |
|
||||||
|
| Use a supplied skill | Follow the [plugin quick start](../README.md#quick-start). The currently exposed skill, `al-code-review`, performs reviews and returns findings. |
|
||||||
|
| Use your own agent or workflow | Supply selected articles as context, as described below, or use the [integration bootstrap](agent-consumption.md#try-a-minimal-integration) to execute BCQuality action skills without the plugin. |
|
||||||
|
|
||||||
|
### Read and reuse an article
|
||||||
|
|
||||||
|
Start with a concern, such as `SetLoadFields`, and search within
|
||||||
|
`microsoft/BCQuality` on GitHub or open its domain folder. For example,
|
||||||
|
[partial-record guidance](../microsoft/knowledge/performance/use-setloadfields-for-partial-records.md)
|
||||||
|
explains the concern and links good/bad samples.
|
||||||
|
|
||||||
|
Before applying an article, read its frontmatter, the small metadata block at
|
||||||
|
the top:
|
||||||
|
|
||||||
|
| Field | How to read it |
|
||||||
|
| --- | --- |
|
||||||
|
| `bc-version` | `[24..]` means BC 24 and later; `[26..28]` means BC 26 through 28; `[all]` means every version. Use your target BC major version, not your extension's version. |
|
||||||
|
| `technologies` | `[al]` means the guidance applies to AL; multiple values identify the technologies the article covers. |
|
||||||
|
| `countries` | `[w1]` means worldwide; a code such as `[dk]` limits the guidance to that localization. |
|
||||||
|
| `application-area` | `[all]` means any application area; a named area narrows applicability. |
|
||||||
|
| `domain` and `keywords` | Help you find the topic; they are not instructions or additional requirements. |
|
||||||
|
|
||||||
|
Read `Description` for context, then `Best Practice` and `Anti Pattern` for the
|
||||||
|
rule and its exceptions. Follow any sample and source links. Do not turn a
|
||||||
|
sample into a production implementation without considering your own context.
|
||||||
|
The [READ reference](../skills/read.md) defines the precise matching rules.
|
||||||
|
|
||||||
|
To use an article with your own agent, give it access to the full article and
|
||||||
|
relevant samples, not just a title or index row. For example, replace the
|
||||||
|
bracketed values in this prompt:
|
||||||
|
|
||||||
|
> Read [article URL or local path] and its linked samples. Apply the relevant
|
||||||
|
> guidance while implementing [task] for BC [major version]. Explain which
|
||||||
|
> guidance you used, cite the article, and identify any missing context.
|
||||||
|
|
||||||
|
A URL only works if the host can retrieve it; otherwise provide the files
|
||||||
|
directly. This is ordinary reuse of knowledge for explanation or code writing,
|
||||||
|
**not a packaged code-generation skill or a complete BCQuality review**.
|
||||||
|
For the structured review process, invoke a supplied skill or follow the
|
||||||
|
integration protocol. The remaining sections describe the review workflow.
|
||||||
|
|
||||||
|
## Hosts and prerequisites
|
||||||
|
|
||||||
|
The [quick start](../README.md#quick-start) documents GitHub Copilot CLI. Use a
|
||||||
|
current CLI release with plugin support and sign in to an account allowed to
|
||||||
|
use it. In an interactive CLI session, `/skills list` should include
|
||||||
|
`al-code-review`; in the terminal, `copilot plugin list` should include
|
||||||
|
`bcquality`.
|
||||||
|
|
||||||
|
Do not assume a CLI installation also installs the plugin into VS Code,
|
||||||
|
another editor, or another agent host. Follow that host's plugin instructions
|
||||||
|
and confirm it discovers `skills/al-code-review/SKILL.md`. Hosts without
|
||||||
|
compatible plugin discovery need an [integration](agent-consumption.md).
|
||||||
|
|
||||||
|
Source review needs access to your files, not a running BC environment.
|
||||||
|
Include `app.json` and any relevant surrounding source. Dependency symbols or
|
||||||
|
a historical baseline may be needed to substantiate particular findings; a
|
||||||
|
review must not invent missing definitions or an earlier version of your app.
|
||||||
|
PowerShell 7 (`pwsh`) accelerates discovery by generating the knowledge index.
|
||||||
|
Without it, folder-based discovery is available and may take longer.
|
||||||
|
|
||||||
|
## Common review requests
|
||||||
|
|
||||||
|
Start a new host session after installing or updating the plugin. Use the
|
||||||
|
skill name explicitly and say what is in scope. Replace example paths and
|
||||||
|
branch names with ones in your project.
|
||||||
|
|
||||||
|
| Task | Example prompt |
|
||||||
|
| --- | --- |
|
||||||
|
| Complete app | Use the installed al-code-review skill to review the complete Business Central app in this folder without changing my source files. Return the complete BCQuality findings report. |
|
||||||
|
| One file | Use the installed al-code-review skill to review `src\CustomerMgt.Codeunit.al` without changing it. Return the complete BCQuality findings report. |
|
||||||
|
| Uncommitted changes | Use the installed al-code-review skill to review my staged and unstaged tracked changes against HEAD, without changing files. Identify any untracked AL files not included in that diff. |
|
||||||
|
| Branch changes | Use the installed al-code-review skill to review changes on this branch since its merge base with `origin/main`. Exclude uncommitted changes and do not edit files. |
|
||||||
|
| Focused review | Use the installed al-code-review skill to review performance in the app in this folder, without changing files. Return the complete performance findings report. |
|
||||||
|
| Supply chain code | Use the installed al-code-review skill to review SCM posting, inventory, reservations, item tracking, and warehouse workflows in this app folder, without changing files. Return the complete Supply Chain Management findings report. |
|
||||||
|
| Agent SDK code | Use the installed al-code-review skill to review Agent SDK implementation and usage in this app folder, without changing files. Return the complete Agents findings report. |
|
||||||
|
|
||||||
|
For Git comparisons, the named base ref must exist locally. If it is missing,
|
||||||
|
fetch the intended branch first. A PR review also requires the host to have
|
||||||
|
the PR's changes and repository access; installing the plugin does not
|
||||||
|
automatically connect it to your PR workflow.
|
||||||
|
|
||||||
|
A complete-folder review considers relevant files recursively, not just
|
||||||
|
modified files. Start in a single app's root for the clearest scope. For a
|
||||||
|
repository containing several apps, name each app folder and review them
|
||||||
|
separately when their target versions or dependencies differ.
|
||||||
|
|
||||||
|
If known, add the target BC major version and localization to the request.
|
||||||
|
Do not use your extension's own `version` as the BC version. Missing
|
||||||
|
applicability context can reduce a finding's confidence or leave a rule out.
|
||||||
|
|
||||||
|
## Reading your results
|
||||||
|
|
||||||
|
The skill returns structured reports. A host may render them as text, a table,
|
||||||
|
or annotations, or show the JSON directly. You can ask the host to explain the
|
||||||
|
returned report without rerunning the review or changing files.
|
||||||
|
|
||||||
|
For example, a performance report could contain this finding:
|
||||||
|
|
||||||
|
| Field | Illustrative value |
|
||||||
|
| --- | --- |
|
||||||
|
| Outcome | `completed` |
|
||||||
|
| Location | `src\CustomerExport.Codeunit.al`, line 42 |
|
||||||
|
| Severity / confidence | `major` / `high` |
|
||||||
|
| Finding | A country filter is evaluated inside the customer loop, so rows that will be discarded are still read. Apply the filter before iterating. |
|
||||||
|
| Guidance | [Apply filters before iterating](../microsoft/knowledge/performance/apply-filters-before-iterating.md), with linked good/bad samples. |
|
||||||
|
|
||||||
|
This illustrates a report, not a guaranteed finding or host screen. Read the
|
||||||
|
referenced article and the surrounding source before accepting a fix.
|
||||||
|
|
||||||
|
### Outcomes
|
||||||
|
|
||||||
|
| Outcome | Meaning and action |
|
||||||
|
| --- | --- |
|
||||||
|
| `completed` | The selected review finished. An empty `findings` list means it found nothing to flag in that scope, not that the app is certified defect-free. |
|
||||||
|
| `not-applicable` | The review did not apply to the supplied input. It is not a clean-review result. |
|
||||||
|
| `no-knowledge` | No applicable knowledge was available. Check scope, target context, and enabled layers. |
|
||||||
|
| `partial` | Some work did not finish. Read `outcome-reason` and the individual reports; do not treat the result as a full pass. |
|
||||||
|
| `failed` | No reliable result from that review. Resolve the reported error before relying on it. |
|
||||||
|
| `no-match` | Routing found no suitable skill. Check the request, input type, and disabled skills. |
|
||||||
|
|
||||||
|
A broad review includes individual domain reports in `sub-results`. Separately
|
||||||
|
dispatched skills return separate reports, so do not mistake the first report
|
||||||
|
for the whole run. Coverage counts describe selected knowledge items evaluated,
|
||||||
|
not a percentage of all possible defects or every rule in the repository.
|
||||||
|
|
||||||
|
For example, this completed **domain** report evaluated one selected knowledge
|
||||||
|
item and found nothing to flag:
|
||||||
|
|
||||||
|
```json
|
||||||
|
{
|
||||||
|
"skill": { "id": "al-performance-review", "version": 1 },
|
||||||
|
"outcome": "completed",
|
||||||
|
"summary": {
|
||||||
|
"counts": { "blocker": 0, "major": 0, "minor": 0, "info": 0 },
|
||||||
|
"coverage": { "worklist-size": 1, "items-evaluated": 1 }
|
||||||
|
},
|
||||||
|
"findings": [],
|
||||||
|
"suppressed": []
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
This is not evidence that other domains ran; their reports must also be present
|
||||||
|
when requested.
|
||||||
|
|
||||||
|
### Severity, confidence, and references
|
||||||
|
|
||||||
|
| Severity | Meaning |
|
||||||
|
| --- | --- |
|
||||||
|
| `blocker` | A platform-level guarantee is violated; the work cannot proceed as-is. |
|
||||||
|
| `major` | A significant defect that should be addressed before merge. |
|
||||||
|
| `minor` | A quality concern; advisory rather than a gate. |
|
||||||
|
| `info` | Concrete context or an observation, not an instruction to change code. |
|
||||||
|
|
||||||
|
Confidence (`high`, `medium`, or `low`) describes the strength of the evidence,
|
||||||
|
not the impact. Missing version or localization context must be disclosed in
|
||||||
|
the finding when conditionally applicable knowledge is used.
|
||||||
|
|
||||||
|
Knowledge-backed findings link to the articles that informed them. Findings
|
||||||
|
from the agent's own reasoning have no knowledge reference (`references: []`);
|
||||||
|
these are advisory, with severity capped at `minor` and confidence at `medium`.
|
||||||
|
The display domain `Agents` means Agent SDK guidance; it is different from
|
||||||
|
`Agent`, the label for the broad coordinator's own cross-cutting observations.
|
||||||
|
Any `suppressed` entries explain knowledge overridden by configuration or
|
||||||
|
layer precedence.
|
||||||
|
|
||||||
|
A report can include a code suggestion. **A suggestion is not an applied
|
||||||
|
change.** Review the explanation first, then request any edits explicitly,
|
||||||
|
for example: "Apply only the filter fix at line 42 from this report." Continue
|
||||||
|
using your normal compilation, analyzer, test, and human-review workflow.
|
||||||
|
|
||||||
|
## Coverage and limits
|
||||||
|
|
||||||
|
The Microsoft broad review composes the Microsoft domains listed below.
|
||||||
|
The Community Agents review is a separate skill selected by the request, not
|
||||||
|
a nested part of that coordinator. All current review leaves accept app
|
||||||
|
folders, files, and diffs; request an Agent SDK review explicitly when that
|
||||||
|
coverage matters and look for its separate report.
|
||||||
|
|
||||||
|
Available knowledge is **not** a promise that every rule will run. Selection
|
||||||
|
depends on the task, target context, enabled layers, and source evidence.
|
||||||
|
A whole-folder review is a current-state snapshot: detecting a published API
|
||||||
|
removal or another comparison-only regression requires an actual baseline.
|
||||||
|
The corpus combines technical AL guidance with targeted functional-domain
|
||||||
|
invariants, not exhaustive functional validation or AppSource certification.
|
||||||
|
|
||||||
|
The SCM leaf owns selected inventory/value, application, reservation, tracking,
|
||||||
|
and warehouse/posting invariants. It prunes unrelated AL using the actual
|
||||||
|
tables, codeunits, fields, and operations in scope; an item caption or a broad
|
||||||
|
`ApplicationArea` alone is not an SCM review signal. Missing workflow context
|
||||||
|
must not be replaced with an assumed posting defect. Manufacturing, assembly,
|
||||||
|
planning, and other supply-chain areas are covered only where an article
|
||||||
|
explicitly names the shared interface or invariant.
|
||||||
|
|
||||||
|
SCM owns Item/Value/Capacity/Warehouse and inventory-application posting
|
||||||
|
records. Pure G/L, customer/vendor/detailed/VAT and financial-only posting
|
||||||
|
mutations belong to Finance, even when that domain is not enabled. Equivalent
|
||||||
|
findings for one inventory-originated posting bypass have one SCM primary
|
||||||
|
owner; distinct independent financial defects remain separate.
|
||||||
|
|
||||||
|
The Finance leaf reviews journal posting, financial ledger changes,
|
||||||
|
applications, and posting-linked dimension handling. It prunes unrelated code
|
||||||
|
at the leaf rather than changing broad-review orchestration. Finance articles
|
||||||
|
use `application-area: [all]` so missing application-area context does not
|
||||||
|
weaken applicable findings; resolved records and operations supply the
|
||||||
|
narrowing. Finance owns financial ledgers, not Item, Value, Capacity, Warehouse,
|
||||||
|
or inventory-application records owned by SCM. It also does not own generic
|
||||||
|
custom-table or master Default Dimension wiring. Request a focused "Finance
|
||||||
|
posting review" when only this domain is needed.
|
||||||
|
|
||||||
|
BCQuality intentionally does not duplicate mechanical diagnostics already
|
||||||
|
enforced by the AL compiler or standard analyzers. Run the consuming app's
|
||||||
|
normal compiler and analyzer pipeline alongside review and authoring. Knowledge
|
||||||
|
may still discuss a diagnostic when BC-specific context is needed to avoid a
|
||||||
|
false positive or choose a correct remediation.
|
||||||
|
|
||||||
|
### Knowledge by domain
|
||||||
|
|
||||||
|
Each article describes one concern. Where samples exist, use its linked
|
||||||
|
`.good.al` and `.bad.al` files. Samples are demonstrations, not a deployable app.
|
||||||
|
|
||||||
|
| Domain | Browse knowledge |
|
||||||
|
| --- | --- |
|
||||||
|
| Agent SDK | [Agents (Community)](../community/knowledge/agents/) |
|
||||||
|
| AppSource | [AppSource](../microsoft/knowledge/appsource/) |
|
||||||
|
| Compatibility | [Breaking changes](../microsoft/knowledge/breaking-changes/) |
|
||||||
|
| Data modeling | [Data modeling](../microsoft/knowledge/data-modeling/) |
|
||||||
|
| Error handling | [Error handling](../microsoft/knowledge/error-handling/) |
|
||||||
|
| Events | [Events](../microsoft/knowledge/events/) |
|
||||||
|
| Finance | [Finance](../microsoft/knowledge/finance/) |
|
||||||
|
| Interfaces | [Interfaces](../microsoft/knowledge/interfaces/) |
|
||||||
|
| Performance | [Performance](../microsoft/knowledge/performance/) |
|
||||||
|
| Privacy | [Privacy](../microsoft/knowledge/privacy/) |
|
||||||
|
| Query objects | [Query](../microsoft/knowledge/query/) |
|
||||||
|
| Reporting | [Reporting](../microsoft/knowledge/reporting/) |
|
||||||
|
| Security | [Security](../microsoft/knowledge/security/) |
|
||||||
|
| Style | [Style](../microsoft/knowledge/style/) |
|
||||||
|
| Supply Chain Management | [SCM](../microsoft/knowledge/scm/) |
|
||||||
|
| Telemetry | [Telemetry](../microsoft/knowledge/telemetry/) |
|
||||||
|
| Testing | [Testing](../microsoft/knowledge/testing/) |
|
||||||
|
| User interface | [UI](../microsoft/knowledge/ui/) |
|
||||||
|
| Upgrades | [Upgrade](../microsoft/knowledge/upgrade/) |
|
||||||
|
| APIs and web services | [Web services](../microsoft/knowledge/web-services/) |
|
||||||
|
|
||||||
|
## Permissions and data
|
||||||
|
|
||||||
|
The review instructions produce findings, not source edits or deployment.
|
||||||
|
The host still controls tool permissions: keep approval prompts enabled and
|
||||||
|
do not grant blanket write or deployment access just to run a review.
|
||||||
|
BCQuality may write its generated `knowledge-index.json` into its own installed
|
||||||
|
directory; that is separate from your app's source.
|
||||||
|
|
||||||
|
BCQuality is content, not an AI service. Your chosen host and model determine
|
||||||
|
where source code is processed, what usage is billed, and which data policies
|
||||||
|
apply. Review those policies before supplying proprietary or customer code.
|
||||||
|
Installing the plugin does not make an online host run locally or offline.
|
||||||
|
|
@ -2,10 +2,41 @@
|
||||||
|
|
||||||
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.
|
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.
|
`review-fixtures.json` contains only global thresholds and optional exceptional overrides. An override may select a different `article`, add context when the generic convention cannot express a scenario, or use an `articles` array when one domain needs explicit regression coverage for several paired articles. Specify either `article` or `articles`, not both. The first selected article retains the stable `<domain>-bad` and `<domain>-good` manifest IDs; additional articles use slug-qualified IDs. Overrides should remain empty in the normal case.
|
||||||
|
|
||||||
|
CI also measures selected paired articles against every effective article that
|
||||||
|
has both AL companions. A changed paired article must be selected by the
|
||||||
|
domain convention or an override. When adding it would not provide a useful
|
||||||
|
deterministic regression, add a narrow `coverageWaivers` entry with its exact
|
||||||
|
article path and a non-empty reason. Waivers are reviewable exceptions, not a
|
||||||
|
substitute for domain coverage. The generated coverage report includes totals
|
||||||
|
and per-domain ratios; the ratio is informational, while changed-file coverage
|
||||||
|
is mandatory.
|
||||||
|
|
||||||
Model-facing preparation hashes case IDs, neutralizes `Good`/`Bad` object-name tokens, and removes full-line sample comments so neither the article slug, domain, nor expected outcome reveals the answer.
|
Model-facing preparation hashes case IDs, neutralizes `Good`/`Bad` object-name tokens, and removes full-line sample comments so neither the article slug, domain, nor expected outcome reveals the answer.
|
||||||
|
|
||||||
|
The SCM `articles` override deliberately selects every rule in the initial
|
||||||
|
functional domain, producing nine positive cases and nine clean controls.
|
||||||
|
Business context is executable: document/status `TestField` guards,
|
||||||
|
calculated-revaluation fields, source-transfer base quantities, warehouse
|
||||||
|
reconciliation steps, and additional-demand promising parameters survive
|
||||||
|
neutralization. Do not move those preconditions into comments or generic
|
||||||
|
"posting" helper names; removing them can turn a real defect into a valid
|
||||||
|
alternative workflow. The clean pairs exercise the supported APIs selected by
|
||||||
|
the same routing cues, not merely unrelated code that contains no SCM tokens.
|
||||||
|
|
||||||
|
The Finance override deliberately covers every paired Finance article, not
|
||||||
|
only the first filename. Its shared context supplies the target version and
|
||||||
|
localization but deliberately omits application area, as production callers
|
||||||
|
often do. Finance applicability must come from its source-surface gate, not
|
||||||
|
an artificial evaluation-only area hint. Scenario prerequisites live in
|
||||||
|
executable AL: the document-balance cases check the template setting, and the
|
||||||
|
VAT cases encode the imported net/VAT/gross totals and applicable VAT mode.
|
||||||
|
Do not move these prerequisites into comments that preparation removes.
|
||||||
|
Clean samples also retain supported operational edits, temporary ledger/set
|
||||||
|
buffers, legitimate entry-number APIs, and reads of individual shortcut
|
||||||
|
dimensions so these exceptions are exercised rather than blanket-excluded.
|
||||||
|
|
||||||
## Validate the corpus
|
## Validate the corpus
|
||||||
|
|
||||||
```powershell
|
```powershell
|
||||||
|
|
@ -14,6 +45,13 @@ pwsh ./tools/Test-ReviewFixtures.ps1 -Root .
|
||||||
|
|
||||||
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.
|
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.
|
||||||
|
|
||||||
|
To reproduce the changed-file gate and emit the same measurable report as CI:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
git diff --name-only origin/main...HEAD | Set-Content .changed-paths.txt
|
||||||
|
pwsh ./tools/Test-ReviewFixtures.ps1 -Root . -ChangedPathsFile .changed-paths.txt -CoverageReportPath .coverage.json
|
||||||
|
```
|
||||||
|
|
||||||
## Run a fast-model evaluation
|
## Run a fast-model evaluation
|
||||||
|
|
||||||
1. Prepare neutral inputs:
|
1. Prepare neutral inputs:
|
||||||
|
|
@ -26,7 +64,7 @@ This credential-free check proves every selected leaf maps to a same-named knowl
|
||||||
|
|
||||||
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.
|
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 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.
|
`request-<domain>.json` files provide optional leaf batches containing every selected case for that domain and identify the selected layer-owned skill path; save those as `result-<domain>.json`. A normal convention-selected domain has one bad/good pair, while an `articles` override contributes one pair per listed article. 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:
|
3. Save only this result shape:
|
||||||
|
|
||||||
|
|
@ -36,7 +74,7 @@ This credential-free check proves every selected leaf maps to a same-named knowl
|
||||||
{
|
{
|
||||||
"id": "case-a1b2c3d4",
|
"id": "case-a1b2c3d4",
|
||||||
"findings": [
|
"findings": [
|
||||||
{ "id": "microsoft/knowledge/appsource/object-affixes-prevent-collisions.md" }
|
{ "id": "microsoft/knowledge/appsource/permission-sets-cover-setup-and-usage-without-super.md" }
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
|
|
||||||
|
|
@ -3,43 +3,188 @@
|
||||||
"selection": "first-paired-al-article",
|
"selection": "first-paired-al-article",
|
||||||
"minimumExpectedRecall": 1.0,
|
"minimumExpectedRecall": 1.0,
|
||||||
"minimumCleanRate": 1.0,
|
"minimumCleanRate": 1.0,
|
||||||
|
"coverageWaivers": [],
|
||||||
"overrides": {
|
"overrides": {
|
||||||
"agents": {
|
"agents": {
|
||||||
"article": "wire-all-three-agent-interfaces"
|
"article": "wire-all-three-agent-interfaces"
|
||||||
},
|
},
|
||||||
"appsource": {
|
|
||||||
"context": "AppSourceCop mandatoryAffixes is configured to ABC."
|
|
||||||
},
|
|
||||||
"breaking-changes": {
|
"breaking-changes": {
|
||||||
"article": "do-not-expose-sensitive-data-through-public-api"
|
"article": "do-not-expose-sensitive-data-through-public-api"
|
||||||
},
|
},
|
||||||
|
"data-modeling": {
|
||||||
|
"articles": [
|
||||||
|
"activate-new-price-calculation-handler-via-onfindsupportedsetup",
|
||||||
|
"check-blocked-in-referencing-code-not-in-master",
|
||||||
|
"code-must-not-change-workdate",
|
||||||
|
"custom-document-dispatch-must-not-bypass-report-selections",
|
||||||
|
"document-line-prices-follow-prices-including-vat",
|
||||||
|
"document-print-and-email-actions-call-report-selections-directly",
|
||||||
|
"extend-find-entries-navigate-for-new-document-types",
|
||||||
|
"extend-price-source-type-must-sync-document-subset-enum",
|
||||||
|
"extend-report-selection-usage-for-new-document-types",
|
||||||
|
"new-price-source-must-add-candidate-and-trigger-recalculation",
|
||||||
|
"pictures-must-use-media-not-blob",
|
||||||
|
"report-barcodes-must-use-barcode-module-and-production-font-name",
|
||||||
|
"table-design-must-match-bc-table-type-conventions",
|
||||||
|
"transferfields-mirrored-fields-must-match-type-and-length"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"error-handling": {
|
||||||
|
"articles": [
|
||||||
|
"collect-validation-errors-with-errorbehavior",
|
||||||
|
"defensive-vs-offensive-code-must-match-blast-radius",
|
||||||
|
"log-writes-must-survive-rollback"
|
||||||
|
]
|
||||||
|
},
|
||||||
"events": {
|
"events": {
|
||||||
"article": "reset-ishandled-only-when-the-value-can-carry-over"
|
"articles": [
|
||||||
|
"reset-ishandled-only-when-the-value-can-carry-over",
|
||||||
|
"changecompany-runs-triggers-in-the-calling-company"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"finance": {
|
||||||
|
"articles": [
|
||||||
|
"apply-ledger-entries-through-application-codeunits",
|
||||||
|
"change-ledger-due-dates-through-entry-edit",
|
||||||
|
"do-not-edit-shared-dimension-sets",
|
||||||
|
"do-not-modify-or-delete-posted-ledger-entries",
|
||||||
|
"normal-vat-journal-amount-includes-vat",
|
||||||
|
"post-ledger-entries-through-posting-codeunits",
|
||||||
|
"preserve-journal-batch-document-balance",
|
||||||
|
"reverse-transactions-by-transaction-number",
|
||||||
|
"write-dimensions-as-dimension-set-entries"
|
||||||
|
],
|
||||||
|
"context": "Target Business Central 28, worldwide standard application (w1). Review each file as an independent source input."
|
||||||
},
|
},
|
||||||
"interfaces": {
|
"interfaces": {
|
||||||
"article": "set-defaultimplementation-on-enum"
|
"article": "set-defaultimplementation-on-enum"
|
||||||
},
|
},
|
||||||
"performance": {
|
"performance": {
|
||||||
"article": "use-isempty-for-existence-check"
|
"articles": [
|
||||||
|
"use-isempty-for-existence-check",
|
||||||
|
"use-get-instead-of-findfirst-on-full-primary-key",
|
||||||
|
"job-queue-category-code-serializes-conflicting-jobs",
|
||||||
|
"job-queue-external-effects-must-be-idempotent",
|
||||||
|
"job-queue-handlers-must-not-require-ui",
|
||||||
|
"job-queue-handlers-must-propagate-failures",
|
||||||
|
"job-queue-on-hold-does-not-stop-running-work",
|
||||||
|
"store-scheduled-task-id-to-avoid-duplicate-tasks",
|
||||||
|
"design-covering-keys-from-read-pattern",
|
||||||
|
"review-overlapping-keys-before-adding-an-index",
|
||||||
|
"setcurrentkey-sets-sort-order-not-index-hint",
|
||||||
|
"preserve-buffered-inserts-by-separating-target-reads",
|
||||||
|
"aggregate-before-persisting-intermediate-results",
|
||||||
|
"cache-repeated-filtered-results-with-explicit-scope",
|
||||||
|
"avoid-repeating-unchanged-validation",
|
||||||
|
"avoid-get-inside-loop-on-large-table",
|
||||||
|
"isempty-before-findset-is-extra-round-trip",
|
||||||
|
"query-results-bypass-primary-key-cache",
|
||||||
|
"calcsums-instead-of-calcfields-in-loop",
|
||||||
|
"use-setautocalcfields-for-per-row-flowfields",
|
||||||
|
"temporary-tables-have-no-database-cost",
|
||||||
|
"use-setloadfields-for-partial-records",
|
||||||
|
"prefer-modifyall-over-per-row-modify",
|
||||||
|
"al-methods-limited-during-write-transactions",
|
||||||
|
"avoid-user-prompts-inside-transactions"
|
||||||
|
]
|
||||||
},
|
},
|
||||||
"privacy": {
|
"privacy": {
|
||||||
"article": "no-pii-in-telemetry-message-string"
|
"article": "no-pii-in-telemetry-message-string"
|
||||||
},
|
},
|
||||||
|
"query": {
|
||||||
|
"articles": [
|
||||||
|
"dataitemtablefilter-cannot-be-overwritten-at-runtime",
|
||||||
|
"reopening-query-resets-cursor-but-keeps-filters",
|
||||||
|
"set-query-filters-before-open",
|
||||||
|
"setfilter-overwrites-query-columnfilter"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"reporting": {
|
||||||
|
"articles": [
|
||||||
|
"clear-report-variable-before-independent-runmodal",
|
||||||
|
"currreport-break-ends-the-current-trigger",
|
||||||
|
"currreport-quit-rolls-back-and-skips-onpostreport",
|
||||||
|
"currreport-skip-does-not-stop-trigger-code",
|
||||||
|
"report-output-in-a-loop-needs-one-client-download",
|
||||||
|
"reportextension-dataitem-trigger-order-is-explicit",
|
||||||
|
"reportextension-report-triggers-run-after-base-triggers",
|
||||||
|
"settableview-cannot-broaden-dataitemtableview",
|
||||||
|
"stop-when-runrequestpage-returns-empty-parameters"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"scm": {
|
||||||
|
"articles": [
|
||||||
|
"post-item-ledger-changes-through-item-journals",
|
||||||
|
"post-revaluation-through-the-item-journal-batch",
|
||||||
|
"change-item-applications-through-posting-routines",
|
||||||
|
"cancel-reservations-through-reservation-management",
|
||||||
|
"transfer-item-tracking-through-source-reservation-codeunits",
|
||||||
|
"reconcile-warehouse-adjustments-with-the-item-ledger",
|
||||||
|
"post-transfers-through-shipment-and-receipt-codeunits",
|
||||||
|
"use-date-aware-availability-for-promising",
|
||||||
|
"carry-out-requisition-actions-through-the-standard-workflow",
|
||||||
|
"derive-base-quantities-through-the-line-unit-of-measure"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"security": {
|
||||||
|
"articles": [
|
||||||
|
"al-has-no-built-in-htmlencode",
|
||||||
|
"do-not-concatenate-external-text-into-setfilter",
|
||||||
|
"exposed-objects-must-be-in-a-permission-set"
|
||||||
|
]
|
||||||
|
},
|
||||||
"style": {
|
"style": {
|
||||||
"article": "label-comment-explains-placeholders"
|
"articles": [
|
||||||
|
"label-comment-explains-placeholders",
|
||||||
|
"dateformula-evaluate-needs-language-independent-literals",
|
||||||
|
"al-comments-must-not-restate-what-code-already-shows",
|
||||||
|
"pages-must-not-contain-business-logic"
|
||||||
|
]
|
||||||
},
|
},
|
||||||
"telemetry": {
|
"telemetry": {
|
||||||
"article": "telemetry-event-id-stable-unique"
|
"article": "telemetry-event-id-stable-unique"
|
||||||
},
|
},
|
||||||
"testing": {
|
"testing": {
|
||||||
"article": "ui-handlers-in-tests"
|
"articles": [
|
||||||
|
"ui-handlers-in-tests",
|
||||||
|
"reset-per-test-state-before-the-isinitialized-guard",
|
||||||
|
"asserterror-needs-expectederror-and-code",
|
||||||
|
"bcpt-scenarios-must-be-app-specific",
|
||||||
|
"commit-shared-test-fixture-inside-lazy-initialize",
|
||||||
|
"given-blocks-must-cover-full-precondition-chain",
|
||||||
|
"table-relation-test-exclude-known-invalid-relations-via-event",
|
||||||
|
"test-feature-scenario-tags",
|
||||||
|
"test-one-when-per-test",
|
||||||
|
"transactionmodel-attribute-governs-test-transactions",
|
||||||
|
"ui-test-codeunit-naming",
|
||||||
|
"use-assert-isfalse-not-asserterror-for-boolean-checks"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"ui": {
|
||||||
|
"articles": [
|
||||||
|
"default-descending-sort-on-historical-pages",
|
||||||
|
"page-design-must-match-bc-page-type-conventions",
|
||||||
|
"page-client-expression-must-not-use-in-list",
|
||||||
|
"rolecenter-permission-gating-must-use-accessbypermission"
|
||||||
|
]
|
||||||
},
|
},
|
||||||
"upgrade": {
|
"upgrade": {
|
||||||
"article": "initvalue-does-not-update-existing-rows",
|
"articles": [
|
||||||
|
"initvalue-does-not-update-existing-rows",
|
||||||
|
"upgrade-tag-logic-must-not-nest-deeply",
|
||||||
|
"no-changecompany-in-upgrade"
|
||||||
|
],
|
||||||
"context": "The extended table existed in the previous app version and already contains rows."
|
"context": "The extended table existed in the previous app version and already contains rows."
|
||||||
},
|
},
|
||||||
"web-services": {
|
"web-services": {
|
||||||
"article": "expose-systemid-as-the-api-key"
|
"articles": [
|
||||||
|
"expose-systemid-as-the-api-key",
|
||||||
|
"handle-httpclient-platform-failure-before-response-access",
|
||||||
|
"check-http-status-before-consuming-response-body",
|
||||||
|
"check-json-null-before-converting-values",
|
||||||
|
"format-exchanged-values-with-standard-format-9",
|
||||||
|
"api-page-least-privilege-write-access"
|
||||||
|
]
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
13
microsoft/knowledge/appsource/file-datatype-saas.bad.al
Normal file
13
microsoft/knowledge/appsource/file-datatype-saas.bad.al
Normal file
|
|
@ -0,0 +1,13 @@
|
||||||
|
codeunit 50104 "Import File Reader"
|
||||||
|
{
|
||||||
|
procedure ImportFile()
|
||||||
|
var
|
||||||
|
ImportFile: File;
|
||||||
|
InStream: InStream;
|
||||||
|
begin
|
||||||
|
ImportFile.WriteMode(false);
|
||||||
|
ImportFile.TextMode(true);
|
||||||
|
ImportFile.Open('C:\Import\data.txt'); // fails in SaaS — no local filesystem
|
||||||
|
ImportFile.CreateInStream(InStream);
|
||||||
|
end;
|
||||||
|
}
|
||||||
24
microsoft/knowledge/appsource/file-datatype-saas.good.al
Normal file
24
microsoft/knowledge/appsource/file-datatype-saas.good.al
Normal file
|
|
@ -0,0 +1,24 @@
|
||||||
|
codeunit 50104 "Import File Reader"
|
||||||
|
{
|
||||||
|
procedure ImportFile()
|
||||||
|
var
|
||||||
|
TempBlob: Codeunit "Temp Blob";
|
||||||
|
FromInStream: InStream;
|
||||||
|
ToOutStream: OutStream;
|
||||||
|
InStream: InStream;
|
||||||
|
begin
|
||||||
|
if not UploadIntoStream('All Files (*.*)|*.*', FromInStream) then
|
||||||
|
exit;
|
||||||
|
|
||||||
|
TempBlob.CreateOutStream(ToOutStream);
|
||||||
|
CopyStream(ToOutStream, FromInStream);
|
||||||
|
|
||||||
|
TempBlob.CreateInStream(InStream);
|
||||||
|
ParseStream(InStream);
|
||||||
|
end;
|
||||||
|
|
||||||
|
local procedure ParseStream(var InStream: InStream)
|
||||||
|
begin
|
||||||
|
// parse InStream content here
|
||||||
|
end;
|
||||||
|
}
|
||||||
28
microsoft/knowledge/appsource/file-datatype-saas.md
Normal file
28
microsoft/knowledge/appsource/file-datatype-saas.md
Normal file
|
|
@ -0,0 +1,28 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: appsource
|
||||||
|
keywords: [file-datatype, saas, onprem, uploadintostream, downloadfromstream, instream, outstream, streaming]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# The File data type's direct I/O methods are OnPrem-only
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
The classic `File` variable type — `Open`/`Create`/`Read`/`Write`/`Close` against a path on the local or server filesystem — is scoped OnPrem-only. Code targeting Business Central Online that calls `File.Open`, `File.Create`, `File.Read`, or `File.Write` fails to compile against a Cloud-scoped project; it does not compile successfully and fail or get silently skipped at runtime. Separately, and regardless of the compile-time scoping, no server/local filesystem path is available to an extension actually running in Business Central Online.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Use the stream-based equivalents: `UploadIntoStream` to read user-selected file content into an `InStream`, and `DownloadFromStream` to write an `OutStream`'s content to a file the user saves. Stage the content in a `TempBlob` between the stream and the rest of the parsing/formatting code.
|
||||||
|
|
||||||
|
See sample: [`file-datatype-saas.good.al`](file-datatype-saas.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Opening a hardcoded or user-supplied filesystem path with the `File` variable type. This is a strong signal the code was written for on-premises only, or copied from material that predates the cloud-first streaming APIs.
|
||||||
|
|
||||||
|
See sample: [`file-datatype-saas.bad.al`](file-datatype-saas.bad.al).
|
||||||
|
|
@ -1,43 +0,0 @@
|
||||||
// Anti-pattern: an own object with no affix. Another app that also defines a
|
|
||||||
// "Loyalty Tier" table cannot be installed alongside this one.
|
|
||||||
table 50379 "Loyalty Tier"
|
|
||||||
{
|
|
||||||
Caption = 'Loyalty Tier';
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; "Code"; Code[20])
|
|
||||||
{
|
|
||||||
Caption = 'Code';
|
|
||||||
}
|
|
||||||
field(10; Description; Text[100])
|
|
||||||
{
|
|
||||||
Caption = 'Description';
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
keys
|
|
||||||
{
|
|
||||||
key(PK; "Code")
|
|
||||||
{
|
|
||||||
Clustered = true;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Anti-pattern (the common half-measure): the extension object carries the
|
|
||||||
// affix, but the field it adds to the standard Customer table does not. That
|
|
||||||
// unaffixed field still collides with any other app that adds "Loyalty Points"
|
|
||||||
// to Customer, and AS0011 flags it.
|
|
||||||
tableextension 50378 "ABC Customer Ext" extends Customer
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(50378; "Loyalty Points"; Integer)
|
|
||||||
{
|
|
||||||
Caption = 'Loyalty Points';
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
@ -1,40 +0,0 @@
|
||||||
// Own object: the affix "ABC" is carried at object-name level.
|
|
||||||
table 50377 "ABC Loyalty Tier"
|
|
||||||
{
|
|
||||||
Caption = 'Loyalty Tier';
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; "Code"; Code[20])
|
|
||||||
{
|
|
||||||
Caption = 'Code';
|
|
||||||
}
|
|
||||||
field(10; Description; Text[100])
|
|
||||||
{
|
|
||||||
Caption = 'Description';
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
keys
|
|
||||||
{
|
|
||||||
key(PK; "Code")
|
|
||||||
{
|
|
||||||
Clustered = true;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Extension of a standard object: the added field is individually affixed,
|
|
||||||
// because the object name (Customer) belongs to the base application.
|
|
||||||
tableextension 50376 "ABC Customer Ext" extends Customer
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(50376; "Loyalty Points ABC"; Integer)
|
|
||||||
{
|
|
||||||
Caption = 'Loyalty Points';
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
@ -1,30 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: appsource
|
|
||||||
keywords: [object-affix, prefix, suffix, as0011, appsourcecop, collision, tableextension, first-party, isv]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Apply a reserved affix to objects and to members added to base objects
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
An AppSource extension must prevent name collisions through its registered affix or, on BC23 and later for objects it owns, a namespace with at least two levels. The affix still applies to every field, key, control, or action added to a base-application object; see `two-level-namespace-replaces-object-affix-not-extension-member-affix.md`. Without either mechanism, two apps that both define a `Loyalty Tier` table cannot coexist, and two apps that add an unaffixed `Loyalty Points` field to `Customer` still collide regardless of their namespaces.
|
|
||||||
|
|
||||||
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).
|
|
||||||
|
|
||||||
See sample: `object-affixes-prevent-collisions.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
An owned object with neither a qualifying namespace nor an affix, an unaffixed extension member, or the common half-measure where the extension object carries the affix but a field it adds to a standard table does not. AS0011 flags the missing collision protection and the field can still collide with another app.
|
|
||||||
|
|
||||||
See sample: `object-affixes-prevent-collisions.bad.al`.
|
|
||||||
|
|
@ -17,10 +17,10 @@ An AppSource app must provide permission sets that let assigned users complete t
|
||||||
|
|
||||||
Trace every setup page, normal page, report, codeunit, and tabledata operation exposed by the app and cover it through assignable role permission sets composed from focused non-assignable sets. Validate setup and representative workflows as a user assigned only those app roles. Grant the minimum required operations; completeness is not a reason to use wildcards.
|
Trace every setup page, normal page, report, codeunit, and tabledata operation exposed by the app and cover it through assignable role permission sets composed from focused non-assignable sets. Validate setup and representative workflows as a user assigned only those app roles. Grant the minimum required operations; completeness is not a reason to use wildcards.
|
||||||
|
|
||||||
See sample: `permission-sets-cover-setup-and-usage-without-super.good.al`.
|
See sample: [`permission-sets-cover-setup-and-usage-without-super.good.al`](permission-sets-cover-setup-and-usage-without-super.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Shipping no permission set, omitting a tabledata or execute grant used by the app's own UI, or instructing users and validators to assign `SUPER` when setup fails. Do not flag a permission-set name that differs from the app name; no such naming requirement exists.
|
Shipping no permission set, omitting a tabledata or execute grant used by the app's own UI, or instructing users and validators to assign `SUPER` when setup fails. Do not flag a permission-set name that differs from the app name; no such naming requirement exists.
|
||||||
|
|
||||||
See sample: `permission-sets-cover-setup-and-usage-without-super.bad.al`.
|
See sample: [`permission-sets-cover-setup-and-usage-without-super.bad.al`](permission-sets-cover-setup-and-usage-without-super.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,37 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: appsource
|
||||||
|
keywords: [version, release, app-json, semver, al-go, appsource]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Update the app version at every release
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
At every release — a branch merged to `main`, a tagged release build, or an AppSource submission — the app's version is consciously updated, not left to the pipeline alone.
|
||||||
|
|
||||||
|
| Version part | Owner | When |
|
||||||
|
|---|---|---|
|
||||||
|
| Major | Developer decision | Breaking change (schema, API, removed objects) |
|
||||||
|
| Minor | Developer decision | Every release with new functionality |
|
||||||
|
| Build / Revision | AL-Go pipeline | Automatic — never hand-edited |
|
||||||
|
|
||||||
|
The app's stable identity is its `id` in `app.json`; the version identifies which release — which code state — of that app is deployed. Two customer environments running "the same" version with different code is an undiagnosable support case. AppSource's actual requirement is strict full-version ordering — the complete version must be greater than the previously submitted version — which an AL-Go-generated build/revision increment can satisfy on its own; AppSource does not require major.minor itself to change. Treating major.minor as a deliberate, human-decided compatibility signal is still valuable practice — it is a statement about what changed that no pipeline can make on its own — just not a platform-enforced requirement.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Before the release merge:
|
||||||
|
app.json: "version": "1.3.0.0" (new functionality -> minor bump, by team convention)
|
||||||
|
AL-Go settings: "repoVersion": "1.3" (where used)
|
||||||
|
Then: feature branch -> main via PR, tag, release.
|
||||||
|
|
||||||
|
"Feature branches never touch the version" and "every merge to main is a release" are workflow choices your team can adopt for compatibility clarity — not something AppSource itself requires.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Branch merged to main and released.
|
||||||
|
app.json still says "version": "1.2.0.0" -- same as the previous release.
|
||||||
|
Two different code states now share one version identity.
|
||||||
|
|
@ -1,22 +0,0 @@
|
||||||
namespace Contoso;
|
|
||||||
|
|
||||||
table 50462 "Rental Agreement"
|
|
||||||
{
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; "No."; Code[20]) { }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
tableextension 50463 "Rental Customer Ext" extends Customer
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(50463; "Loyalty Points"; Integer)
|
|
||||||
{
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
@ -1,22 +0,0 @@
|
||||||
namespace Contoso.Rentals;
|
|
||||||
|
|
||||||
table 50460 "Rental Agreement"
|
|
||||||
{
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; "No."; Code[20]) { }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
tableextension 50461 "Rental Customer Ext" extends Customer
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(50461; "Loyalty Points RNT"; Integer)
|
|
||||||
{
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
@ -1,28 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [23..]
|
|
||||||
domain: appsource
|
|
||||||
keywords: [namespace, two-level, affix, prefix, suffix, as0011, tableextension, pageextension, false-positive]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# A two-level namespace replaces an object affix, not an extension-member affix
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
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. 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` 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`.
|
|
||||||
|
|
@ -17,10 +17,10 @@ Access is a decision about what you are willing to support forever. The moment a
|
||||||
|
|
||||||
Start everything `local` or `internal` and promote a member to `public` only when you have decided to support it as a stable contract. Expose a small, intentional surface — the supported entry point — and keep validation, posting, and helper routines `internal` for in-app reuse or `local` when single-object. Do not drop `[Scope('OnPrem')]` without intent, since that too widens the contract. Every public member is a maintenance commitment; spend them deliberately.
|
Start everything `local` or `internal` and promote a member to `public` only when you have decided to support it as a stable contract. Expose a small, intentional surface — the supported entry point — and keep validation, posting, and helper routines `internal` for in-app reuse or `local` when single-object. Do not drop `[Scope('OnPrem')]` without intent, since that too widens the contract. Every public member is a maintenance commitment; spend them deliberately.
|
||||||
|
|
||||||
See sample: `choose-access-modifiers-deliberately.good.al`.
|
See sample: [`choose-access-modifiers-deliberately.good.al`](choose-access-modifiers-deliberately.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Declaring every procedure `public` by default, so internal helpers like `ValidateOrder` and `PostOrder` become a de-facto API that consumers bind to and that can no longer be changed freely. Detection: an object where implementation-detail procedures carry no access modifier or are `public` without a reason to support them externally. Default them to `internal`/`local` and make only the intended entry point public.
|
Declaring every procedure `public` by default, so internal helpers like `ValidateOrder` and `PostOrder` become a de-facto API that consumers bind to and that can no longer be changed freely. Detection: an object where implementation-detail procedures carry no access modifier or are `public` without a reason to support them externally. Default them to `internal`/`local` and make only the intended entry point public.
|
||||||
|
|
||||||
See sample: `choose-access-modifiers-deliberately.bad.al`.
|
See sample: [`choose-access-modifiers-deliberately.bad.al`](choose-access-modifiers-deliberately.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ Deleting or renaming a published procedure (or object) in a single release is a
|
||||||
|
|
||||||
When a published procedure is superseded, keep it in place and mark it `[Obsolete('Use CalculateNetAmount instead.', '25.0')]`, where the message names the replacement and the tag records when the method became obsolete. Have the obsolete member forward to the new one so behavior is preserved during the window. Only after the deprecation window has elapsed should a later release delete the method. For an object or field, use `Pending` during the warning window and `Removed` afterward.
|
When a published procedure is superseded, keep it in place and mark it `[Obsolete('Use CalculateNetAmount instead.', '25.0')]`, where the message names the replacement and the tag records when the method became obsolete. Have the obsolete member forward to the new one so behavior is preserved during the window. Only after the deprecation window has elapsed should a later release delete the method. For an object or field, use `Pending` during the warning window and `Removed` afterward.
|
||||||
|
|
||||||
See sample: `deprecate-public-members-with-the-obsolete-lifecycle.good.al`.
|
See sample: [`deprecate-public-members-with-the-obsolete-lifecycle.good.al`](deprecate-public-members-with-the-obsolete-lifecycle.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Renaming or deleting the published `CalcNet` procedure in place — replacing it with `CalculateNetAmount` and nothing else — so consumers calling `CalcNet` break immediately with no deprecation notice. Detection: a previously shipped non-`local` procedure that vanished or was renamed between versions with no `[Obsolete]` marker left behind during a prior warning window. Do not suggest `ObsoleteState = Removed` for a method; that property belongs to supported object and element types.
|
Renaming or deleting the published `CalcNet` procedure in place — replacing it with `CalculateNetAmount` and nothing else — so consumers calling `CalcNet` break immediately with no deprecation notice. Detection: a previously shipped non-`local` procedure that vanished or was renamed between versions with no `[Obsolete]` marker left behind during a prior warning window. Do not suggest `ObsoleteState = Removed` for a method; that property belongs to supported object and element types.
|
||||||
|
|
||||||
See sample: `deprecate-public-members-with-the-obsolete-lifecycle.bad.al`.
|
See sample: [`deprecate-public-members-with-the-obsolete-lifecycle.bad.al`](deprecate-public-members-with-the-obsolete-lifecycle.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -19,10 +19,10 @@ This rule governs procedures that dependents *call*. An event publisher — a pr
|
||||||
|
|
||||||
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.
|
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.
|
||||||
|
|
||||||
See sample: `do-not-change-published-procedure-signatures.good.al`.
|
See sample: [`do-not-change-published-procedure-signatures.good.al`](do-not-change-published-procedure-signatures.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## 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. 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.
|
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`.
|
See sample: [`do-not-change-published-procedure-signatures.bad.al`](do-not-change-published-procedure-signatures.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ Every member you make publicly reachable becomes a contract you must keep — an
|
||||||
|
|
||||||
Keep secrets in `internal` or `local` members, and prefer the `SecretText` type so the value cannot be read back or logged. Where callers genuinely need a credential, pass it inward (a setter) rather than handing it outward (a getter). Public API should return only non-sensitive data — a masked reference, a status, a business identifier — never the raw secret. Treat each public member as a lasting commitment and keep the security-sensitive surface as small as possible.
|
Keep secrets in `internal` or `local` members, and prefer the `SecretText` type so the value cannot be read back or logged. Where callers genuinely need a credential, pass it inward (a setter) rather than handing it outward (a getter). Public API should return only non-sensitive data — a masked reference, a status, a business identifier — never the raw secret. Treat each public member as a lasting commitment and keep the security-sensitive surface as small as possible.
|
||||||
|
|
||||||
See sample: `do-not-expose-sensitive-data-through-public-api.good.al`.
|
See sample: [`do-not-expose-sensitive-data-through-public-api.good.al`](do-not-expose-sensitive-data-through-public-api.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
A public `GetAccessToken()` that returns the raw token (or an event parameter carrying a credential to all subscribers), turning a secret into a de-facto public API any dependent can consume. Detection: a non-`local` procedure, event parameter, or global variable that surfaces a token, password, key, or other credential. Keep the secret internal and expose only non-sensitive data.
|
A public `GetAccessToken()` that returns the raw token (or an event parameter carrying a credential to all subscribers), turning a secret into a de-facto public API any dependent can consume. Detection: a non-`local` procedure, event parameter, or global variable that surfaces a token, password, key, or other credential. Keep the secret internal and expose only non-sensitive data.
|
||||||
|
|
||||||
See sample: `do-not-expose-sensitive-data-through-public-api.bad.al`.
|
See sample: [`do-not-expose-sensitive-data-through-public-api.bad.al`](do-not-expose-sensitive-data-through-public-api.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -17,10 +17,10 @@ A member carrying `[Obsolete]`, or wrapped in a `#if not CLEANxx` conditional-co
|
||||||
|
|
||||||
Leave obsolete members exactly as they are and implement against the current, supported replacement. New logic — a surcharge calculation, an event publisher, a hook — belongs on the live API (`GetUnitPrice`), never inside the deprecated `GetPrice` or behind a `#if not CLEAN25` guard. If the replacement does not yet exist, create it as a first-class member and build there. The obsolete code should only shrink over time, not accrete new behavior.
|
Leave obsolete members exactly as they are and implement against the current, supported replacement. New logic — a surcharge calculation, an event publisher, a hook — belongs on the live API (`GetUnitPrice`), never inside the deprecated `GetPrice` or behind a `#if not CLEAN25` guard. If the replacement does not yet exist, create it as a first-class member and build there. The obsolete code should only shrink over time, not accrete new behavior.
|
||||||
|
|
||||||
See sample: `do-not-modify-code-already-marked-obsolete.good.al`.
|
See sample: [`do-not-modify-code-already-marked-obsolete.good.al`](do-not-modify-code-already-marked-obsolete.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Adding a surcharge calculation inside the `[Obsolete]` `GetPrice` procedure, or behind a `#if not CLEAN25` block, so the new behavior is wired to code that will be removed when `CLEAN25` is enabled. Detection: new statements, event declarations, or dependencies introduced inside an `[Obsolete]`-marked member or a `#if not CLEANxx` region. Move the logic onto the supported replacement instead.
|
Adding a surcharge calculation inside the `[Obsolete]` `GetPrice` procedure, or behind a `#if not CLEAN25` block, so the new behavior is wired to code that will be removed when `CLEAN25` is enabled. Detection: new statements, event declarations, or dependencies introduced inside an `[Obsolete]`-marked member or a `#if not CLEANxx` region. Move the logic onto the supported replacement instead.
|
||||||
|
|
||||||
See sample: `do-not-modify-code-already-marked-obsolete.bad.al`.
|
See sample: [`do-not-modify-code-already-marked-obsolete.bad.al`](do-not-modify-code-already-marked-obsolete.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -1,9 +0,0 @@
|
||||||
// This published object previously used namespace Contoso.Rentals.
|
|
||||||
namespace Contoso.RentalManagement;
|
|
||||||
|
|
||||||
codeunit 50467 "Rental Agreement Mgt."
|
|
||||||
{
|
|
||||||
procedure CreateAgreement()
|
|
||||||
begin
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,8 +0,0 @@
|
||||||
namespace Contoso.Rentals;
|
|
||||||
|
|
||||||
codeunit 50466 "Rental Agreement Mgt."
|
|
||||||
{
|
|
||||||
procedure CreateAgreement()
|
|
||||||
begin
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,26 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [23..]
|
|
||||||
domain: breaking-changes
|
|
||||||
keywords: [namespace, published-object, dependency, breaking-change, as0007, compile-time-identity]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Treat a published namespace as part of object identity
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
AL resolves an object by namespace and name. Once an app ships and dependent extensions compile against that identity, changing the namespace breaks their references even when the object name and ID stay unchanged. AppSourceCop AS0007 rejects changing the namespace of published objects; namespaces are therefore not a cosmetic folder-like label that can be reorganized after release.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Choose a globally meaningful namespace before first publication and keep it stable. Add new functional areas beneath that structure without moving existing published objects. If an identity must move, use the platform's supported move/obsoletion lifecycle rather than a source-only namespace rename.
|
|
||||||
|
|
||||||
See sample: `namespace-is-part-of-published-object-identity.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Changing `namespace Contoso.Rentals;` to `namespace Contoso.RentalManagement;` as a cleanup while leaving the object name and ID untouched. Every dependent `using` directive and qualified reference targets the old identity and stops compiling.
|
|
||||||
|
|
||||||
See sample: `namespace-is-part-of-published-object-identity.bad.al`.
|
|
||||||
|
|
@ -17,10 +17,10 @@ A shipped table field carries both a source-level contract and persisted data. R
|
||||||
|
|
||||||
Keep the old field's ID, name, and type unchanged. Add the replacement as a separate field under an unused ID, then mark the old field `ObsoleteState = Pending` with an `ObsoleteReason` that names the replacement and an `ObsoleteTag` recording the obsoletion version. Keep the old field readable so an upgrade codeunit can copy its data during the deprecation window. Move it to `ObsoleteState = Removed` only in a later release, after the window has passed and data has migrated.
|
Keep the old field's ID, name, and type unchanged. Add the replacement as a separate field under an unused ID, then mark the old field `ObsoleteState = Pending` with an `ObsoleteReason` that names the replacement and an `ObsoleteTag` recording the obsoletion version. Keep the old field readable so an upgrade codeunit can copy its data during the deprecation window. Move it to `ObsoleteState = Removed` only in a later release, after the window has passed and data has migrated.
|
||||||
|
|
||||||
See sample: `obsolete-table-fields-instead-of-deleting-them.good.al`.
|
See sample: [`obsolete-table-fields-instead-of-deleting-them.good.al`](obsolete-table-fields-instead-of-deleting-them.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Renaming published `Email` to `Contact Email` with the same ID violates the compatibility contract and AS0005, even though the retained ID does not itself imply a fresh empty column. Deleting `Email` or changing its ID additionally risks losing its stored values. Detection: any previously shipped field whose name changes at the same ID, or whose original ID disappears without the unchanged field being retained as `Pending` and its data migrated to a separate replacement field.
|
Renaming published `Email` to `Contact Email` with the same ID violates the compatibility contract and AS0005, even though the retained ID does not itself imply a fresh empty column. Deleting `Email` or changing its ID additionally risks losing its stored values. Detection: any previously shipped field whose name changes at the same ID, or whose original ID disappears without the unchanged field being retained as `Pending` and its data migrated to a separate replacement field.
|
||||||
|
|
||||||
See sample: `obsolete-table-fields-instead-of-deleting-them.bad.al`.
|
See sample: [`obsolete-table-fields-instead-of-deleting-them.bad.al`](obsolete-table-fields-instead-of-deleting-them.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,9 @@
|
||||||
|
codeunit 50103 "Order Confirmation Notifier"
|
||||||
|
{
|
||||||
|
procedure Send(ToAddress: Text; Subject: Text; Body: Text)
|
||||||
|
var
|
||||||
|
Mail: Codeunit Mail;
|
||||||
|
begin
|
||||||
|
Mail.CreateMessage(ToAddress, '', '', Subject, Body, false, false);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,11 @@
|
||||||
|
codeunit 50103 "Order Confirmation Notifier"
|
||||||
|
{
|
||||||
|
procedure Send(ToAddress: Text; Subject: Text; Body: Text)
|
||||||
|
var
|
||||||
|
Email: Codeunit Email;
|
||||||
|
EmailMessage: Codeunit "Email Message";
|
||||||
|
begin
|
||||||
|
EmailMessage.Create(ToAddress, Subject, Body, true);
|
||||||
|
Email.Send(EmailMessage, Enum::"Email Scenario"::Default);
|
||||||
|
end;
|
||||||
|
}
|
||||||
28
microsoft/knowledge/breaking-changes/prefer-email-module.md
Normal file
28
microsoft/knowledge/breaking-changes/prefer-email-module.md
Normal file
|
|
@ -0,0 +1,28 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: breaking-changes
|
||||||
|
keywords: [email, codeunit-mail, email-message, email-scenario, email-account, smtp, sending-email]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Send email through the Email module, not Codeunit Mail (397)
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Older AL code sends email by calling `Codeunit Mail (397)`. Business Central's current extensibility model is a different, richer object set — `Codeunit Email`, `Codeunit "Email Message"`, `enum "Email Scenario"`, and the `Email Account`/`Email Connector` interface (Microsoft 365, Current User, SMTP, or a custom connector). `Codeunit "Email Message"` is the in-memory object you build the message on; it is not itself the persisted Sent/Outbox/Draft record — that storage is managed separately once the message is queued or sent. New code built on `Codeunit Mail` inherits its SMTP-era, single-connector assumptions and leaves no Sent/Outbox trail behind.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Build on `Codeunit Email` and `Codeunit "Email Message"`. Route the message through an `Email Scenario` so different document types can use different accounts without the calling code needing to know which account that is, and get a tracked Sent/Outbox/Draft record for free.
|
||||||
|
|
||||||
|
See sample: [`prefer-email-module.good.al`](prefer-email-module.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Calling `Codeunit Mail`'s `CreateMessage`. It still compiles and runs, but current `Codeunit Mail`'s own implementation of `CreateMessage` no longer sends anything by itself — it only raises integration events for a legacy subscriber to act on — so building new code on it means depending on whatever compatibility shim happens to still be wired up, with no first-class connector selection and no queryable Sent/Outbox/Draft record. `Send` and `GetErrorDesc` are not current members of `Codeunit Mail` at all; do not reference them.
|
||||||
|
|
||||||
|
See sample: [`prefer-email-module.bad.al`](prefer-email-module.bad.al).
|
||||||
|
|
@ -0,0 +1,74 @@
|
||||||
|
enumextension 50102 "Sample Price Calc Handler Ext" extends "Price Calculation Handler"
|
||||||
|
{
|
||||||
|
value(50102; "Sample Special Price")
|
||||||
|
{
|
||||||
|
Caption = 'Sample Special Price';
|
||||||
|
Implementation = "Price Calculation" = "Sample Price Calc - Special";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Demonstration-only AL: every method below is stubbed. This article is
|
||||||
|
// about activating a handler through OnFindSupportedSetup, not about the
|
||||||
|
// "Price Calculation" interface's own pricing logic.
|
||||||
|
codeunit 50103 "Sample Price Calc - Special" implements "Price Calculation"
|
||||||
|
{
|
||||||
|
procedure Init(LineWithPrice: Interface "Line With Price"; PriceCalculationSetup: Record "Price Calculation Setup")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure GetLine(var Line: Variant)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure ApplyDiscount()
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure ApplyPrice(CalledByFieldNo: Integer)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure CountDiscount(ShowAll: Boolean) Result: Integer
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure CountPrice(ShowAll: Boolean) Result: Integer
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure FindDiscount(var TempPriceListLine: Record "Price List Line"; ShowAll: Boolean) Found: Boolean
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure FindPrice(var TempPriceListLine: Record "Price List Line"; ShowAll: Boolean) Found: Boolean
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure IsDiscountExists(ShowAll: Boolean) Result: Boolean
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure IsPriceExists(ShowAll: Boolean) Result: Boolean
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure PickDiscount()
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure PickPrice()
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure ShowPrices(var TempPriceListLine: Record "Price List Line")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
||||||
|
// WRONG: no subscriber to Price Calculation Mgt.'s OnFindSupportedSetup.
|
||||||
|
// "Sample Special Price" is a real, working implementation of the Price
|
||||||
|
// Calculation interface - it simply has no Price Calculation Setup row
|
||||||
|
// naming it, so Price Calculation Mgt. never selects it for any sale,
|
||||||
|
// purchase, or job line, whether through the Default fallback or through
|
||||||
|
// a "Dtld. Price Calculation Setup" row. It ships invisible until someone
|
||||||
|
// notices and configures a setup row for it by hand.
|
||||||
|
|
@ -0,0 +1,90 @@
|
||||||
|
enumextension 50102 "Sample Price Calc Handler Ext" extends "Price Calculation Handler"
|
||||||
|
{
|
||||||
|
value(50102; "Sample Special Price")
|
||||||
|
{
|
||||||
|
Caption = 'Sample Special Price';
|
||||||
|
Implementation = "Price Calculation" = "Sample Price Calc - Special";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Demonstration-only AL: every method below is stubbed. This article is
|
||||||
|
// about activating a handler through OnFindSupportedSetup, not about the
|
||||||
|
// "Price Calculation" interface's own pricing logic.
|
||||||
|
codeunit 50103 "Sample Price Calc - Special" implements "Price Calculation"
|
||||||
|
{
|
||||||
|
procedure Init(LineWithPrice: Interface "Line With Price"; PriceCalculationSetup: Record "Price Calculation Setup")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure GetLine(var Line: Variant)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure ApplyDiscount()
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure ApplyPrice(CalledByFieldNo: Integer)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure CountDiscount(ShowAll: Boolean) Result: Integer
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure CountPrice(ShowAll: Boolean) Result: Integer
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure FindDiscount(var TempPriceListLine: Record "Price List Line"; ShowAll: Boolean) Found: Boolean
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure FindPrice(var TempPriceListLine: Record "Price List Line"; ShowAll: Boolean) Found: Boolean
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure IsDiscountExists(ShowAll: Boolean) Result: Boolean
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure IsPriceExists(ShowAll: Boolean) Result: Boolean
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure PickDiscount()
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure PickPrice()
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure ShowPrices(var TempPriceListLine: Record "Price List Line")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50104 "Sample Price Calc Setup Install"
|
||||||
|
{
|
||||||
|
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Price Calculation Mgt.", 'OnFindSupportedSetup', '', false, false)]
|
||||||
|
local procedure AddSampleSpecialPriceSetup(var TempPriceCalculationSetup: Record "Price Calculation Setup" temporary)
|
||||||
|
begin
|
||||||
|
TempPriceCalculationSetup.Init();
|
||||||
|
TempPriceCalculationSetup.Code := 'SAMPLE-SPECIAL';
|
||||||
|
TempPriceCalculationSetup.Method := TempPriceCalculationSetup.Method::"Lowest Price";
|
||||||
|
TempPriceCalculationSetup.Type := TempPriceCalculationSetup.Type::Sale;
|
||||||
|
TempPriceCalculationSetup."Asset Type" := TempPriceCalculationSetup."Asset Type"::" ";
|
||||||
|
TempPriceCalculationSetup.Implementation := TempPriceCalculationSetup.Implementation::"Sample Special Price";
|
||||||
|
TempPriceCalculationSetup.Enabled := true;
|
||||||
|
// Default := true here because this row is meant as the fallback
|
||||||
|
// for Method = Lowest Price / Type = Sale / Asset Type = " " (all)
|
||||||
|
// - the combination Price Calculation Mgt.'s FindSetup selects via
|
||||||
|
// its own SetRange(Default, true) branch when no "Dtld. Price
|
||||||
|
// Calculation Setup" row names a more specific match. A handler
|
||||||
|
// meant to be picked only through such a specific, explicit
|
||||||
|
// detailed-setup row would not need Default := true at all.
|
||||||
|
TempPriceCalculationSetup.Default := true;
|
||||||
|
TempPriceCalculationSetup.Insert();
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,98 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: data-modeling
|
||||||
|
keywords: [price-calculation, price-calculation-handler, price-calculation-setup, integration-event, pricing]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Activate a new Price Calculation Handler through OnFindSupportedSetup, not just by implementing it
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
`enum 7011 "Price Calculation Handler"` (`implements "Price
|
||||||
|
Calculation"`) is how a new pricing engine plugs into Business Central —
|
||||||
|
extend the enum with a value pointing at a codeunit that implements the
|
||||||
|
`Price Calculation` interface. That alone does not make the new handler
|
||||||
|
usable on any document. `codeunit 7001 "Price Calculation Mgt."` decides
|
||||||
|
which handler applies to a given line by looking up `table 7006 "Price
|
||||||
|
Calculation Setup"`, a table of `(Code, Method, Type, Asset Type,
|
||||||
|
Implementation, Enabled, Default)` rows populated at startup by its own
|
||||||
|
`OnFindSupportedSetup` event — every implementation codeunit is expected
|
||||||
|
to subscribe to that event and insert its own setup row(s). A handler
|
||||||
|
enum value with no matching setup row is real and selectable in the enum
|
||||||
|
itself, but never chosen for any actual sale, purchase, or job line,
|
||||||
|
because `Price Calculation Mgt.` has no setup row that names it.
|
||||||
|
|
||||||
|
`FindSetup` resolves a handler in two stages, and only the second one
|
||||||
|
looks at `Default`. It first asks `codeunit 7004 "Price Calculation Dtld.
|
||||||
|
Setup"` to match the line against `table 7008 "Dtld. Price Calculation
|
||||||
|
Setup"` ("Detailed Price Calculation Setup", keyed to an exact
|
||||||
|
`Method`/`Type`/`Asset Type`/`Source`/`Asset No.` combination via its own
|
||||||
|
`"Setup Code"`); on a match it does `PriceCalculationSetup.Get(...
|
||||||
|
"Setup Code")` directly, with no `Default` filter. Only when no detailed
|
||||||
|
row matches does it fall back to `SetRange(Default, true)` plus
|
||||||
|
`SetRange(Method, ...)` to pick the one catch-all row for that
|
||||||
|
combination. A row without `Default := true` is invisible to *that*
|
||||||
|
fallback, but not invisible outright — a detailed-setup row can still
|
||||||
|
select it by naming its `Code`. A row whose `Method` matches neither path
|
||||||
|
is invisible either way — same symptom, different cause.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Ship a new `Price Calculation Handler` value together with an
|
||||||
|
`OnFindSupportedSetup` subscriber that inserts at least one `Price
|
||||||
|
Calculation Setup` record naming it as the `Implementation`, for the
|
||||||
|
relevant `Method` (e.g. `"Lowest Price"`), `Type` (`Sale`/`Purchase`), and
|
||||||
|
`Asset Type`. `Default := true` is required only when this row is the
|
||||||
|
*fallback* for that combination — the row `FindSetup`'s own
|
||||||
|
`SetRange(Default, true)` branch selects when no more specific setup
|
||||||
|
applies. A handler meant to be selected only for specific customers or
|
||||||
|
items should instead be reachable through a matching `"Dtld. Price
|
||||||
|
Calculation Setup"` row; `FindSetup` resolves that before it ever checks
|
||||||
|
`Default`, so it needs no `Default := true`.
|
||||||
|
|
||||||
|
See sample: [`activate-new-price-calculation-handler-via-onfindsupportedsetup.good.al`](activate-new-price-calculation-handler-via-onfindsupportedsetup.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Extending `Price Calculation Handler` and implementing the `Price
|
||||||
|
Calculation` interface, without subscribing to `OnFindSupportedSetup` to
|
||||||
|
insert a setup record. The new handler exists, compiles, and can even be
|
||||||
|
selected manually if a user creates their own `Price Calculation Setup`
|
||||||
|
row through the UI — but ships with no default row, so it's never active
|
||||||
|
for anyone until someone notices it's missing and configures it by hand.
|
||||||
|
|
||||||
|
See sample: [`activate-new-price-calculation-handler-via-onfindsupportedsetup.bad.al`](activate-new-price-calculation-handler-via-onfindsupportedsetup.bad.al).
|
||||||
|
|
||||||
|
## Source
|
||||||
|
|
||||||
|
BCApps (`src/Layers/W1/BaseApp/Pricing/Calculation/`):
|
||||||
|
`PriceCalculationHandler.Enum.al` (`enum 7011 "Price Calculation Handler"
|
||||||
|
implements "Price Calculation"`); `PriceCalculationMgt.Codeunit.al`
|
||||||
|
(`OnFindSupportedSetup(var TempPriceCalculationSetup: Record "Price
|
||||||
|
Calculation Setup" temporary)`, and `FindSetup(...): Boolean`, which
|
||||||
|
first calls `PriceCalculationDtldSetup.FindSetup(DtldPriceCalcSetup)` and
|
||||||
|
on a match does `PriceCalculationSetup.Get(... "Setup Code")` with no
|
||||||
|
`Default` filter — only on failure does it fall back to
|
||||||
|
`SetRange(Enabled, true)`, `SetRange(Default, true)`, `SetRange(Method,
|
||||||
|
...)`); `PriceCalculationSetup.Table.al` (`table 7006 "Price Calculation
|
||||||
|
Setup"`: `Code`, `Method`, `Type`, `"Asset Type"`, `Implementation`,
|
||||||
|
`Enabled`, `Default`); `PriceCalculationDtldSetup.Codeunit.al` (`codeunit
|
||||||
|
7004 "Price Calculation Dtld. Setup"`, `FindSetup(var DtldPriceCalcSetup:
|
||||||
|
Record "Dtld. Price Calculation Setup"): Boolean`, matching progressively
|
||||||
|
looser `Source Group`/`Source No.`/`Asset Type`/`Asset No.` combinations —
|
||||||
|
never `Default`); `DtldPriceCalculationSetup.Table.al` (`table 7008 "Dtld.
|
||||||
|
Price Calculation Setup"`, Caption "Detailed Price Calculation Setup",
|
||||||
|
`"Setup Code"` relates to `"Price Calculation Setup".Code where(Enabled =
|
||||||
|
const(true))` — no `Default` condition).
|
||||||
|
|
||||||
|
Microsoft Learn, "Extending Price Calculations": "Each codeunit that
|
||||||
|
implements the Price Calculation interface must subscribe to the
|
||||||
|
OnFindSupportedSetup() event... to fill the price calculation setup
|
||||||
|
table." Same article: "You can enter detailed setup records for
|
||||||
|
non-default setup lines... If a matching setup is found its
|
||||||
|
implementation is used... If there is no matching setup exception, we
|
||||||
|
use the default implementation."
|
||||||
|
(https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/devenv-extending-best-price-calculations)
|
||||||
|
|
@ -19,10 +19,10 @@ Putting the block check inside the master's own `OnInsert`/`OnModify` does nothi
|
||||||
|
|
||||||
The referencing line validates `Master.TestField(Blocked, false)` in `OnValidate` of the reference field and re-checks before posting. The master table stays logic-free on `Blocked`.
|
The referencing line validates `Master.TestField(Blocked, false)` in `OnValidate` of the reference field and re-checks before posting. The master table stays logic-free on `Blocked`.
|
||||||
|
|
||||||
See sample: `check-blocked-in-referencing-code-not-in-master.good.al`.
|
See sample: [`check-blocked-in-referencing-code-not-in-master.good.al`](check-blocked-in-referencing-code-not-in-master.good.al).
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
The block check sits in the master's own `OnModify`/`OnInsert` (so referencing and posting proceed unchecked), or there is no check at all on the referencing side.
|
The block check sits in the master's own `OnModify`/`OnInsert` (so referencing and posting proceed unchecked), or there is no check at all on the referencing side.
|
||||||
|
|
||||||
See sample: `check-blocked-in-referencing-code-not-in-master.bad.al`.
|
See sample: [`check-blocked-in-referencing-code-not-in-master.bad.al`](check-blocked-in-referencing-code-not-in-master.bad.al).
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,22 @@
|
||||||
|
codeunit 50101 "Meter Jnl.-Post"
|
||||||
|
{
|
||||||
|
procedure Post(var MeterJnlLine: Record "Meter Journal Line")
|
||||||
|
var
|
||||||
|
MeterLedgEntry: Record "Meter Ledger Entry";
|
||||||
|
begin
|
||||||
|
// Validation, Journal access, and posting all mixed in one routine.
|
||||||
|
if not Confirm('Post journal lines?') then
|
||||||
|
exit;
|
||||||
|
|
||||||
|
if MeterJnlLine.FindSet() then
|
||||||
|
repeat
|
||||||
|
if MeterJnlLine.Quantity = 0 then
|
||||||
|
Error('Quantity must not be zero.');
|
||||||
|
|
||||||
|
MeterLedgEntry.Init();
|
||||||
|
MeterLedgEntry.TransferFields(MeterJnlLine);
|
||||||
|
MeterLedgEntry.Insert();
|
||||||
|
MeterJnlLine.Delete();
|
||||||
|
until MeterJnlLine.Next() = 0;
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,42 @@
|
||||||
|
codeunit 50100 "Meter Jnl.-Check Line"
|
||||||
|
{
|
||||||
|
procedure CheckLine(var MeterJnlLine: Record "Meter Journal Line")
|
||||||
|
begin
|
||||||
|
// Reads setup/dimension data only, shows no UI beyond errors.
|
||||||
|
if MeterJnlLine.Quantity = 0 then
|
||||||
|
Error('Quantity must not be zero.');
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50101 "Meter Jnl.-Post Line"
|
||||||
|
{
|
||||||
|
procedure PostLine(var MeterJnlLine: Record "Meter Journal Line")
|
||||||
|
var
|
||||||
|
MeterLedgEntry: Record "Meter Ledger Entry";
|
||||||
|
begin
|
||||||
|
// Posts exactly one journal line; never touches the Journal table.
|
||||||
|
MeterLedgEntry.Init();
|
||||||
|
MeterLedgEntry.TransferFields(MeterJnlLine);
|
||||||
|
MeterLedgEntry.Insert();
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50102 "Meter Jnl.-Post Batch"
|
||||||
|
{
|
||||||
|
procedure PostBatch(var MeterJnlLine: Record "Meter Journal Line")
|
||||||
|
var
|
||||||
|
CheckLine: Codeunit "Meter Jnl.-Check Line";
|
||||||
|
PostLine: Codeunit "Meter Jnl.-Post Line";
|
||||||
|
begin
|
||||||
|
if MeterJnlLine.FindSet() then
|
||||||
|
repeat
|
||||||
|
CheckLine.CheckLine(MeterJnlLine);
|
||||||
|
until MeterJnlLine.Next() = 0;
|
||||||
|
|
||||||
|
if MeterJnlLine.FindSet() then
|
||||||
|
repeat
|
||||||
|
PostLine.PostLine(MeterJnlLine);
|
||||||
|
MeterJnlLine.Delete();
|
||||||
|
until MeterJnlLine.Next() = 0;
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,28 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: data-modeling
|
||||||
|
keywords: [posting-routine, check-line, post-line, post-batch, companion-codeunit, yes-no-wrapper, journal]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Split posting routines into Check Line / Post Line / Post Batch
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Business Central's own journal-based posting routines consistently follow a three-codeunit split, each with one primary responsibility — `Codeunit "Gen. Jnl.-Check Line"` / `"Gen. Jnl.-Post Line"` / `"Gen. Jnl.-Post Batch"` for the general journal, and the same `<Journal>-Check Line` / `<Journal>-Post Line` / `<Journal>-Post Batch` shape repeated for Item, Resource, Job, Fixed Asset, Insurance, and Cost Accounting journals: `Check Line` validates one line, `Post Line` posts exactly one journal line — `Gen. Jnl.-Post Line` itself can write more than one G/L Entry per call (a balancing entry, VAT, currency rounding, deferrals), so "exactly one line" describes its input, not a one-entry-out guarantee — and `Post Batch` loops both across the journal. A document posting routine (posting one document at a time) calls `Post Line` directly and skips `Post Batch`. This is the standard shape to evaluate a new journal-based posting routine against, not a platform-enforced constraint — a routine with a genuinely different transaction/reuse shape may legitimately organize itself differently, and the three codeunits' responsibilities are a useful default split, not a guarantee that every implementation keeps them non-overlapping. But a new routine that blurs this split without a specific reason either misses functionality other code expects to call directly, or exposes an interaction surface it shouldn't.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
`Check Line` reads setup/dimension data only on its first call and shows no UI beyond errors. `Post Line` only operates on the record passed to it — never the Journal table — so it can be called directly by other posting code, including a document posting routine. `Post Batch` is the only one of the three that reads and updates the Journal table, and it is the only one invoked from the Post action on a journal page. A `-Post` document codeunit is never called directly from a page; a page calls a `-Post (Yes/No)` confirmation wrapper instead, so the same `-Post` codeunit can also run unattended from a batch-posting report.
|
||||||
|
|
||||||
|
See sample: [`check-post-line-batch-pattern.good.al`](check-post-line-batch-pattern.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
A single monolithic posting codeunit that reads the Journal table, validates lines, writes ledger entries, and shows confirmation dialogs all in one procedure. It cannot be reused by another posting routine without fabricating journal records, and it cannot run unattended because it insists on user interaction.
|
||||||
|
|
||||||
|
See sample: [`check-post-line-batch-pattern.bad.al`](check-post-line-batch-pattern.bad.al).
|
||||||
|
|
@ -0,0 +1,9 @@
|
||||||
|
codeunit 50101 "Batch Job Runner"
|
||||||
|
{
|
||||||
|
procedure AdvanceToNextBusinessDay()
|
||||||
|
begin
|
||||||
|
// Anti-pattern: repurposes the user's session WorkDate as a
|
||||||
|
// scratch variable for unrelated business logic.
|
||||||
|
WorkDate(CalcDate('<1D>', WorkDate()));
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,11 @@
|
||||||
|
codeunit 50100 "Posting Date Helper"
|
||||||
|
{
|
||||||
|
procedure GetDefaultPostingDate(): Date
|
||||||
|
var
|
||||||
|
PostingDate: Date;
|
||||||
|
begin
|
||||||
|
// Read the work date to default a value; never write to it.
|
||||||
|
PostingDate := WorkDate();
|
||||||
|
exit(PostingDate);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,58 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: data-modeling
|
||||||
|
keywords: [workdate, session-setting, user-control, side-effect]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Application code must not change the WorkDate
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
The work date is a per-user session setting the user controls from the
|
||||||
|
client (the date shown in the top-right corner, used to default posting
|
||||||
|
dates and date filters). Business logic unrelated to that setting must not
|
||||||
|
call `WorkDate(NewDate)` as a side effect of doing something else — that
|
||||||
|
silently changes what the user sees and defaults to for the rest of their
|
||||||
|
session, a surprising, hard-to-trace behavior change the user never asked
|
||||||
|
for and has no visibility into. This is not a blanket ban on the setter
|
||||||
|
itself: BCApps' own demo-data generators legitimately save the current
|
||||||
|
work date, set a specific one to backdate the data they create, and
|
||||||
|
restore it afterward (see `CreateDemoEDocsBE.Codeunit.al`'s
|
||||||
|
`WorkDate(SampleInvoiceDate)` / `WorkDate(SavedWorkDate)` pair), and test
|
||||||
|
codeunits routinely set `WorkDate` deliberately to control the date context
|
||||||
|
a test runs under (hundreds of calls across BCApps' test suite, for
|
||||||
|
example `SustainabilityPostingTest.Codeunit.al`). Both are the code's
|
||||||
|
*actual purpose*, not a side effect of something unrelated.
|
||||||
|
|
||||||
|
This is a call-direction distinction for the read side: reading the
|
||||||
|
current work date via `WorkDate` (or `WorkDate()` with no argument) is
|
||||||
|
always fine.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Read the work date to default a value. Only write to it when changing it
|
||||||
|
*is* the operation being performed — implementing the user's own
|
||||||
|
work-date/settings action, or a test or demo-data routine that deliberately
|
||||||
|
establishes a date context (saving and restoring the prior value if the
|
||||||
|
routine must leave the session as it found it). Business logic that exists
|
||||||
|
to do something else must never write `WorkDate` as an incidental side
|
||||||
|
effect; if a calculation needs a specific date, pass or compute that date
|
||||||
|
as a local variable instead.
|
||||||
|
|
||||||
|
See sample: [`code-must-not-change-workdate.good.al`](code-must-not-change-workdate.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Setting the work date from within a codeunit, report, or page action whose
|
||||||
|
purpose is unrelated to the user's date preference — for example, a
|
||||||
|
posting or calculation routine that calls `WorkDate(SomeDate)` to make its
|
||||||
|
own logic simpler. This changes session state the user owns for the
|
||||||
|
duration of a call that was never about the work date, and never restores
|
||||||
|
it. This is a different case from a test or demo-data routine explicitly
|
||||||
|
declaring a date context: the anti-pattern is unrelated logic silently
|
||||||
|
mutating state it does not own, not the setter form itself.
|
||||||
|
|
||||||
|
See sample: [`code-must-not-change-workdate.bad.al`](code-must-not-change-workdate.bad.al).
|
||||||
|
|
@ -0,0 +1,20 @@
|
||||||
|
codeunit 50102 "Sample Posted Invoice Send"
|
||||||
|
{
|
||||||
|
procedure SendPostedInvoice(SalesInvoiceHeader: Record "Sales Invoice Header")
|
||||||
|
var
|
||||||
|
Customer: Record Customer;
|
||||||
|
begin
|
||||||
|
Customer.Get(SalesInvoiceHeader."Bill-to Customer No.");
|
||||||
|
Customer.TestField("E-Mail");
|
||||||
|
|
||||||
|
// WRONG: the report is hardcoded instead of resolved through the
|
||||||
|
// registered "S.Invoice" usage in Report Selections. This alone is
|
||||||
|
// the defect - no hand-built email is needed for it: a Report
|
||||||
|
// Selections row or a per-customer "Document Layouts" override
|
||||||
|
// that points this usage at a different report or layout is
|
||||||
|
// silently ignored, and the only way to change what this code
|
||||||
|
// prints is a code change and a new release.
|
||||||
|
SalesInvoiceHeader.SetRecFilter();
|
||||||
|
Report.RunModal(Report::"Standard Sales - Invoice", false, false, SalesInvoiceHeader);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,31 @@
|
||||||
|
codeunit 50102 "Sample Posted Invoice Send"
|
||||||
|
{
|
||||||
|
procedure SendPostedInvoice(SalesInvoiceHeader: Record "Sales Invoice Header")
|
||||||
|
var
|
||||||
|
ReportSelections: Record "Report Selections";
|
||||||
|
ReportDistributionMgt: Codeunit "Report Distribution Management";
|
||||||
|
begin
|
||||||
|
// Custom validation specific to this dispatch stays here...
|
||||||
|
CheckReadyToSend(SalesInvoiceHeader);
|
||||||
|
|
||||||
|
// ...but dispatch goes through the registered usage. "S.Invoice"
|
||||||
|
// resolves to a report built on "Sales Invoice Header" (by default
|
||||||
|
// report 1306 "Standard Sales - Invoice"), so the record passed in
|
||||||
|
// matches what the selected report expects, and per-account
|
||||||
|
// report/layout overrides and email attachment/body configuration
|
||||||
|
// on Report Selections all apply automatically.
|
||||||
|
SalesInvoiceHeader.SetRecFilter();
|
||||||
|
ReportSelections.SendEmailToCust(
|
||||||
|
"Report Selection Usage"::"S.Invoice".AsInteger(), SalesInvoiceHeader, SalesInvoiceHeader."No.",
|
||||||
|
ReportDistributionMgt.GetFullDocumentTypeText(SalesInvoiceHeader), true,
|
||||||
|
SalesInvoiceHeader."Bill-to Customer No.");
|
||||||
|
end;
|
||||||
|
|
||||||
|
local procedure CheckReadyToSend(SalesInvoiceHeader: Record "Sales Invoice Header")
|
||||||
|
var
|
||||||
|
Customer: Record Customer;
|
||||||
|
begin
|
||||||
|
Customer.Get(SalesInvoiceHeader."Bill-to Customer No.");
|
||||||
|
Customer.TestField("E-Mail");
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,69 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: data-modeling
|
||||||
|
keywords: [report-selections, document-layouts, custom-report-layout, email-attachment, bespoke-dispatch]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Custom document dispatch must not bypass Report Selections
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
A codeunit that hardcodes which report to run (`Report.RunModal(MyReportId, ...)`),
|
||||||
|
or builds its own email directly, instead of registering the document
|
||||||
|
through `table 77 "Report Selections"` and calling its own
|
||||||
|
Print/Email procedures, works for the one case it was written for — and
|
||||||
|
loses everything the platform's registry provides for free. Either
|
||||||
|
bypass is a defect on its own: a hardcoded report ignores the registered
|
||||||
|
report and any per-account layout override even when no email is
|
||||||
|
involved, and a hand-built email ignores the registry's attachment and
|
||||||
|
email-body configuration even when the report itself came from it. `Report
|
||||||
|
Selections` carries its own attachment/email-body configuration per usage
|
||||||
|
(`"Use for Email Attachment"`, `"Use for Email Body"`, `"Email Body Layout
|
||||||
|
Code"`, `"Email Body Layout Type"`), plus a separate per-usage layout
|
||||||
|
override, `"Custom Report Layout Code"`, and
|
||||||
|
`table 9657 "Custom Report Selection"` (the "Document Layouts" page on the
|
||||||
|
Customer/Vendor card) lets one specific account override the report or
|
||||||
|
layout without touching code at all. None of that exists for a document
|
||||||
|
whose dispatch was hand-rolled: there is no registry row to point
|
||||||
|
"Document Layouts" at, so an admin who goes looking for where to change
|
||||||
|
this document's layout — the same place they'd look for every other
|
||||||
|
document in the system — finds nothing, because the document was never
|
||||||
|
registered there.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Register the document under a `Report Selection Usage` value (see
|
||||||
|
`extend-report-selection-usage-for-new-document-types.md`) and dispatch
|
||||||
|
through `Report Selections`' own Print/Email procedures (see
|
||||||
|
`document-print-and-email-actions-call-report-selections-directly.md`),
|
||||||
|
even when the surrounding business logic — which counterparty to use,
|
||||||
|
what validation must pass before sending — is genuinely specific to the
|
||||||
|
document. Custom logic belongs around the call to `Report Selections`,
|
||||||
|
not instead of it.
|
||||||
|
|
||||||
|
See sample: [`custom-document-dispatch-must-not-bypass-report-selections.good.al`](custom-document-dispatch-must-not-bypass-report-selections.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
A codeunit that runs a hardcoded report ID, or builds its own email
|
||||||
|
message directly, for a document that has (or should have) a
|
||||||
|
`Report Selections` usage — each is independently a bypass, and the
|
||||||
|
sample shows the first on its own. It works for the default case, but the report/layout cannot be changed per account
|
||||||
|
without a code change and a new release, and the document is invisible to
|
||||||
|
"Document Layouts" — the standard place every other document's
|
||||||
|
distribution is configured.
|
||||||
|
|
||||||
|
See sample: [`custom-document-dispatch-must-not-bypass-report-selections.bad.al`](custom-document-dispatch-must-not-bypass-report-selections.bad.al).
|
||||||
|
|
||||||
|
## Source
|
||||||
|
|
||||||
|
BCApps `ReportSelections.Table.al` (table 77 — field 7,
|
||||||
|
`"Custom Report Layout Code"`; fields 19–26 for email attachment/body
|
||||||
|
configuration; `SendEmailToCust`/`PrintWithDialogForCust` as the
|
||||||
|
registry-backed dispatch entry points) and
|
||||||
|
`CustomReportSelection.Table.al` (table 9657, the per-account override
|
||||||
|
backing the "Document Layouts" page) — both under
|
||||||
|
`src/Layers/W1/BaseApp/Foundation/Reporting/`.
|
||||||
|
|
@ -0,0 +1,13 @@
|
||||||
|
table 50100 "Course"
|
||||||
|
{
|
||||||
|
fields
|
||||||
|
{
|
||||||
|
field(1; "No."; Code[20]) { }
|
||||||
|
field(10; "Global Dimension 1 Code"; Code[20])
|
||||||
|
{
|
||||||
|
// No CaptionClass, no OnValidate call into DimensionManagement.
|
||||||
|
// Accepts any value; never becomes a Default Dimension record.
|
||||||
|
TableRelation = "Dimension Value".Code;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,89 @@
|
||||||
|
// Master data: Default Dimension records, no Dimension Set ID field.
|
||||||
|
table 50100 "Course"
|
||||||
|
{
|
||||||
|
fields
|
||||||
|
{
|
||||||
|
field(1; "No."; Code[20]) { }
|
||||||
|
field(10; "Global Dimension 1 Code"; Code[20])
|
||||||
|
{
|
||||||
|
CaptionClass = '1,1,1';
|
||||||
|
TableRelation = "Dimension Value".Code where(
|
||||||
|
"Global Dimension No." = const(1), Blocked = const(false));
|
||||||
|
|
||||||
|
trigger OnValidate()
|
||||||
|
var
|
||||||
|
DimMgt: Codeunit DimensionManagement;
|
||||||
|
begin
|
||||||
|
DimMgt.ValidateDimValueCode(1, "Global Dimension 1 Code");
|
||||||
|
DimMgt.SaveDefaultDim(Database::Course, "No.", 1, "Global Dimension 1 Code");
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
trigger OnDelete()
|
||||||
|
var
|
||||||
|
DimMgt: Codeunit DimensionManagement;
|
||||||
|
begin
|
||||||
|
DimMgt.DeleteDefaultDim(Database::Course, "No.");
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Transactional/document data: a single Dimension Set ID, inherited from the
|
||||||
|
// related master record and overridable via shortcut dimension fields.
|
||||||
|
table 50101 "Course Registration Header"
|
||||||
|
{
|
||||||
|
fields
|
||||||
|
{
|
||||||
|
field(1; "No."; Code[20]) { }
|
||||||
|
field(2; "Customer No."; Code[20])
|
||||||
|
{
|
||||||
|
TableRelation = Customer;
|
||||||
|
|
||||||
|
trigger OnValidate()
|
||||||
|
begin
|
||||||
|
UpdateDimensionSetID();
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
field(10; "Shortcut Dimension 1 Code"; Code[20])
|
||||||
|
{
|
||||||
|
CaptionClass = '1,1,1';
|
||||||
|
TableRelation = "Dimension Value".Code where(
|
||||||
|
"Global Dimension No." = const(1), Blocked = const(false));
|
||||||
|
|
||||||
|
trigger OnValidate()
|
||||||
|
var
|
||||||
|
DimMgt: Codeunit DimensionManagement;
|
||||||
|
begin
|
||||||
|
DimMgt.ValidateShortcutDimValues(1, "Shortcut Dimension 1 Code", "Dimension Set ID");
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
field(480; "Dimension Set ID"; Integer)
|
||||||
|
{
|
||||||
|
Editable = false;
|
||||||
|
TableRelation = "Dimension Set Entry"."Dimension Set ID";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
local procedure UpdateDimensionSetID()
|
||||||
|
var
|
||||||
|
Customer: Record Customer;
|
||||||
|
DimMgt: Codeunit DimensionManagement;
|
||||||
|
DefaultDimSource: List of [Dictionary of [Integer, Code[20]]];
|
||||||
|
GlobalDim2Code: Code[20];
|
||||||
|
begin
|
||||||
|
// Recompute from scratch (InheritFromDimSetID = 0) whether or not the
|
||||||
|
// customer lookup succeeds. Passing the existing "Dimension Set ID"
|
||||||
|
// here would inherit dimensions from whichever record the document
|
||||||
|
// was previously linked to, and exiting early on a failed Get would
|
||||||
|
// leave that same stale data in place — both defeat the point of
|
||||||
|
// this procedure. Clearing the shortcut field and recomputing with
|
||||||
|
// an empty source list (when the customer doesn't exist) correctly
|
||||||
|
// clears the document's dimensions instead of leaving old ones.
|
||||||
|
"Shortcut Dimension 1 Code" := '';
|
||||||
|
if Customer.Get("Customer No.") then
|
||||||
|
DimMgt.AddDimSource(DefaultDimSource, Database::Customer, "Customer No.");
|
||||||
|
"Dimension Set ID" :=
|
||||||
|
DimMgt.GetDefaultDimID(
|
||||||
|
DefaultDimSource, '', "Shortcut Dimension 1 Code", GlobalDim2Code, 0, 0);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,35 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: data-modeling
|
||||||
|
keywords: [dimensions, dimensionmanagement, global-dimension, shortcut-dimension, default-dimension, validatedimvaluecode, getdefaultdimid]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Wire dimension support through DimensionManagement, not ad hoc fields
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Adding dimension support to a custom table is not just a matter of adding a `Code[20]` field, and master tables and document/transactional tables wire into `Codeunit "Dimension Management"` through two different models — treating them as one mechanism is itself the mistake this article corrects:
|
||||||
|
|
||||||
|
- **Master data** (a custom master table, e.g. "Course") persists **Default Dimension** records: each shortcut dimension field validates through `ValidateDimValueCode`, then the result is saved via `SaveDefaultDim`, and `DeleteDefaultDim` removes them again in `OnDelete`. Both `ValidateDimValueCode` and `SaveDefaultDim` take the shortcut dimension *number* (1-8, matching `General Ledger Setup`'s "Shortcut Dimension N Code" fields) as their first/third argument respectively — not the AL field ID of the table field being validated. The master record itself carries no `Dimension Set ID` field.
|
||||||
|
- **Transactional/document data** (a custom document or journal-line table) carries a single **`Dimension Set ID`** field — a pointer to a shared, deduplicated set of dimension values in `Dimension Set Entry`, assembled from whatever the document inherited plus whatever the user overrode. A document does not acquire that ID by calling `SaveDefaultDim`; it builds a source list with `AddDimSource` (naming the related master table and its key, e.g. `Database::Customer`), then calls `GetDefaultDimID` to compute a new `Dimension Set ID` that inherits the master's Default Dimension records. Editing a shortcut dimension field directly on the document validates through `ValidateShortcutDimValues`, which updates the same `Dimension Set ID` in place rather than writing a separate Default Dimension record.
|
||||||
|
|
||||||
|
Skipping the model that actually matches the table's kind produces a field that looks correct in the designer but silently fails to save, validate, or carry through to postings — or, for a document, one that never picks up the customer's/vendor's own dimensions at all.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
For a master table, validate each shortcut dimension field through `ValidateDimValueCode`, save the result with `SaveDefaultDim`, and delete the matching Default Dimension records in `OnDelete`.
|
||||||
|
|
||||||
|
For a document table, when the field that attaches the document to a master record changes (e.g. `Customer No.`), call `AddDimSource` naming that master table and key, then `GetDefaultDimID` to compute the document's new `Dimension Set ID`, inheriting the master's Default Dimension records. Pass `0` for `GetDefaultDimID`'s `InheritFromDimSetID` argument in this case — passing the document's *existing* `Dimension Set ID` instead inherits whatever dimensions were already in it, so a value the previous linked record supplied can survive into the new one even where the new record has no default for that dimension. Run this same recompute — clear the shortcut field, call `GetDefaultDimID` with no source added — when the lookup on the new key fails (blank or an invalid value), too: exiting early instead leaves the previous record's dimensions in place, which is the same staleness bug the `InheritFromDimSetID = 0` rule exists to prevent. Validate the document's own Shortcut Dimension fields through `ValidateShortcutDimValues`, which updates that same `Dimension Set ID` in place rather than persisting a separate Default Dimension record.
|
||||||
|
|
||||||
|
See sample: [`dimension-management-wiring.good.al`](dimension-management-wiring.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Adding a dimension-looking field with only a `TableRelation` to Dimension Value, and no call into `DimensionManagement` at all. The field accepts input but never becomes a real Default Dimension record, so it does not validate against blocked values and does not flow into postings.
|
||||||
|
|
||||||
|
See sample: [`dimension-management-wiring.bad.al`](dimension-management-wiring.bad.al).
|
||||||
|
|
@ -0,0 +1,14 @@
|
||||||
|
table 50100 "Period Stats"
|
||||||
|
{
|
||||||
|
fields
|
||||||
|
{
|
||||||
|
field(1; "Period Start"; Date) { }
|
||||||
|
field(2; Flow; Enum "Some Flow") { }
|
||||||
|
}
|
||||||
|
keys
|
||||||
|
{
|
||||||
|
// Table already shipped with key(PK; "Period Start").
|
||||||
|
// Adding Flow here breaks every upgrade with AS0009.
|
||||||
|
key(PK; Flow, "Period Start") { Clustered = true; }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,24 @@
|
||||||
|
table 50100 "Period Stats"
|
||||||
|
{
|
||||||
|
fields
|
||||||
|
{
|
||||||
|
field(1; "Period Start"; Date) { }
|
||||||
|
}
|
||||||
|
keys
|
||||||
|
{
|
||||||
|
key(PK; "Period Start") { Clustered = true; }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
table 50101 "Period Stats By Flow"
|
||||||
|
{
|
||||||
|
fields
|
||||||
|
{
|
||||||
|
field(1; "Period Start"; Date) { }
|
||||||
|
field(2; Flow; Enum "Some Flow") { }
|
||||||
|
}
|
||||||
|
keys
|
||||||
|
{
|
||||||
|
key(PK; Flow, "Period Start") { Clustered = true; }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,28 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: data-modeling
|
||||||
|
keywords: [primary-key, clustered-key, table-design, appsource, breaking-change, schema-upgrade]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Never Change a Published Table's Primary or Clustered Key Field List
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Once a table has shipped — to AppSource, or to any customer environment that has already upgraded onto it — its primary key, and any other key marked `Clustered = true`, is frozen. This includes adding a field to the key, not only removing or reordering one: Business Central identifies existing rows by their key value, so any change to which fields compose that key invalidates every row already stored under the old shape, and the platform's upgrade validation rejects it outright (`AS0009`). This is easy to trip over because it doesn't look like the well-known "don't delete a field" mistake — the field being added is often brand new, and folding a new discriminating dimension straight into the existing key feels like the natural, un-denormalized way to model it. On an unpublished table that is correct; on a published one it is a breaking schema change regardless of which direction the field list changed, and there is no in-place fix once the upgrade is rejected, only reverting the key to its published shape.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Leave a published table's key exactly as shipped. Model a new discriminating dimension as a separate table with its own key instead of adding a field to the existing key, and branch orchestration code by the new dimension rather than filtering one shared table on an extra key field.
|
||||||
|
|
||||||
|
See sample: [`do-not-change-primary-key.good.al`](do-not-change-primary-key.good.al).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Adding a field to a published table's primary or clustered key to distinguish a new case. This fails AppSource validation or any customer upgrade with `AS0009` as soon as rows already exist under the old key shape, whether the field is being added, removed, or reordered.
|
||||||
|
|
||||||
|
See sample: [`do-not-change-primary-key.bad.al`](do-not-change-primary-key.bad.al).
|
||||||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Add table
Add a link
Reference in a new issue