.claude-plugin/marketplace.json

@@ -9,10 +9,7 @@
       "name": "bcquality",
       "source": "./",
       "description": "Business Central AL quality knowledge base and review skills, packaged as an installable plugin. Ships the entire BCQuality tree (skills, knowledge, tools) so the Entry routing protocol runs against the installed clone.",
-      "version": "0.1.0",
-      "skills": [
-        "./skills/bcquality-al-review/"
-      ]
+      "version": "0.1.0"
     }
   ]
 }

.claude-plugin/plugin.json

@@ -5,17 +5,5 @@
   "author": {
     "name": "microsoft/BCQuality",
     "url": "https://github.com/microsoft/BCQuality"
-  },
-  "repository": "https://github.com/microsoft/BCQuality",
-  "license": "MIT",
-  "keywords": [
-    "bc",
-    "al",
-    "business-central",
-    "code-review",
-    "quality"
-  ],
-  "skills": [
-    "./skills/bcquality-al-review/"
-  ]
+  }
 }

microsoft/knowledge/breaking-changes/relocating-a-field-to-a-tableextension-is-not-a-deletion.md

@@ -1,18 +0,0 @@
----
-bc-version: [all]
-domain: breaking-changes
-keywords: [table-field, tableextension, relocation, field-id, obsoletestate, breaking-change, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# Relocating a field to a tableextension in the same app is not a deletion
-
-## Description
-
-Moving a field out of a base-table definition (or a base-app layer modification of one) into a tableextension that `extends` the same table, within the same app and keeping the same field ID and name, is a relocation — not a deletion or a rename. After the move the field still exists on the table: `Rec."Field Name"` and the field ID resolve exactly as before, so dependent extensions that reference the field continue to compile. Nothing in the field's public contract is removed or renamed, so the deprecation lifecycle that protects a genuinely removed field does not apply. LLM reviewers frequently misread the two-sided diff — the field disappearing from the base object and reappearing in the tableextension — as a shipped field being deleted and illegally re-added under the same ID, and demand `ObsoleteState = Pending` staging that this refactor does not need.
-
-## Best Practice
-
-Recognize a field that is removed from a base table (or base-app layer) and re-declared in a tableextension of the same table, with the same field ID and name, as a same-app relocation. Do not flag it as a deleted or renamed shipped field, and do not require `ObsoleteState = Pending`, `ObsoleteReason`, `ObsoleteTag`, or a deprecation window for the move itself. The `obsolete-table-fields-instead-of-deleting-them` and `obsolete-pending-to-removed-staging` rules apply to fields that leave the table's contract entirely, not to fields relocated within the same app under an unchanged ID.

microsoft/knowledge/error-handling/unchecked-get-throws-when-record-not-found.md

@@ -1,26 +0,0 @@
----
-bc-version: [all]
-domain: error-handling
-keywords: [get, record-not-found, runtime-error, return-value, boolean-method, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# An unchecked Record.Get raises an error when the record is missing; it is not silently ignored
-
-## Description
-
-`Record.Get` returns a Boolean, but its behavior when no record is found depends on whether the return value is consumed. When the return value is used — inside `if Rec.Get(...) then`, or assigned to a variable — a missing record yields `false` and execution continues. When `Rec.Get(...)` is called as a bare statement and the return value is not used, the platform raises a runtime "record not found" error if the record does not exist. A bare `Rec.Get(Key)` therefore acts as an assertion that the record exists: it does not swallow or silently ignore a missing record. This mirrors other AL find methods, where an unconsumed return value lets the platform enforce the not-found error.
-
-## Best Practice
-
-Do not claim that a `Record.Get` whose return value is unused silently ignores a missing record or hides an error. Treat a bare `Rec.Get(...)` statement as an intentional existence assertion that already throws when the record is absent. Recommend an explicit existence check only when the surrounding logic must continue gracefully rather than error out.
-
-## Anti Pattern
-
-Flagging a bare `Rec.Get(Key)` statement as a defect because "the return value is ignored, so a missing record is swallowed", or recommending it be wrapped in `if Rec.Get(...) then ... else Error(...)` to "handle the not-found case" — the unchecked call already raises an error when the record is missing.
-
-## See also
-
-- `ignored-tryfunction-return-disables-try-semantics.md` — a different case where ignoring a Boolean return value changes behavior.

microsoft/knowledge/events/adding-a-parameter-to-an-event-is-not-a-breaking-change.md

@@ -1,18 +0,0 @@
----
-bc-version: [all]
-domain: events
-keywords: [event-parameters, signature, subscriber-binding, backward-compatibility, integration-event, breaking-change, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# Adding a parameter to an event is not a breaking change
-
-## Description
-
-Adding a parameter to an existing event publisher does not break existing subscribers. AL binds a subscriber to a publisher by the event name, and the subscriber's parameter list only has to be a subset of the publisher's, matched by name and type. A subscriber that does not declare the new parameter keeps compiling and keeps binding — it simply ignores the addition. This holds for `IntegrationEvent` and `BusinessEvent` publishers, and even more plainly for `local` events. Appending the new parameter at the end keeps the change a clean, reviewable addition (see `add-new-event-parameters-at-the-end`). LLM reviewers often misreport the mere presence of a new event parameter as a "breaking event signature change" that breaks subscribers, which is incorrect.
-
-## Best Practice
-
-Do not flag the addition of a parameter to an event publisher as a breaking or signature-breaking change, and do not claim it breaks existing subscribers. Genuine, separate concerns are covered by their own rules — a parameter inserted in the middle of the list rather than appended (`add-new-event-parameters-at-the-end`), or a parameter that carries no meaningful value — and should be raised on those grounds, not framed as a backward-compatibility break.

microsoft/knowledge/performance/onaftergetcurrrecord-is-not-per-row.md

@@ -1,26 +0,0 @@
----
-bc-version: [all]
-domain: performance
-keywords: [onaftergetcurrrecord, onaftergetrecord, calcfields, n-plus-one, page-lifecycle, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# Database work in OnAfterGetCurrRecord is not a per-row or N+1 cost
-
-## Description
-
-`OnAfterGetCurrRecord` fires only when the current/active record changes — typically once when the page opens and once each time the user selects a different row — not once for every row rendered in a list. Database work placed there, such as `CalcFields`, `Get`, or a lookup, therefore runs a bounded number of times driven by user navigation, not multiplied by the number of visible rows. This is unlike `OnAfterGetRecord`, which fires once per row as the page loads records and can create a genuine N+1 pattern. Reviewers sometimes see `CalcFields` or a database call inside a page trigger and assume it runs for every row; the trigger name determines whether that assumption holds.
-
-## Best Practice
-
-Before flagging `CalcFields`, `Get`, or a similar database call in a page trigger as a per-row or N+1 problem, confirm the trigger is `OnAfterGetRecord`, which runs per row. Do not flag the same work in `OnAfterGetCurrRecord`: that trigger runs on current-record change, not for every displayed row.
-
-## Anti Pattern
-
-Reporting `CalcFields` or another database call inside `OnAfterGetCurrRecord` as an N+1 or per-row performance defect, or recommending it be moved out "to avoid running once per row". The trigger does not run per row.
-
-## See also
-
-- `calcfields-in-both-getrecord-triggers-is-not-redundant.md` — the lifecycle distinction between the two triggers.

microsoft/knowledge/performance/primary-key-get-in-loop-is-transaction-cached.md

@@ -1,24 +0,0 @@
----
-bc-version: [all]
-domain: performance
-keywords: [get, primary-key, record-cache, transaction, n-plus-one, dictionary-cache, over-engineering, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# A primary-key Get() in a per-row helper is not an N+1 to cache manually
-
-## Description
-
-The Business Central server caches primary-key reads within a transaction. Repeated `Record.Get(<primary key>)` calls for the same key are served from that cache rather than re-queried, so a guarded `if not Rec.Get(...) then exit;` inside a per-row helper is not a genuine N+1 pattern. When each row legitimately carries a distinct key — for example one `Bin Content` row per bin, so `Bin.Get` and `BinType.Get` see a different bin each iteration — the `Get` must run per row regardless, and there is nothing to hoist.
-
-Reviewers sometimes see two `Get` calls inside a routine that runs once per row and recommend wrapping them in a `Dictionary` cache. That is over-engineering: it duplicates the server's built-in record cache, adds state that must be invalidated, and breaks the surrounding extension's established pattern of direct guarded `Get` calls.
-
-## Best Practice
-
-Treat a primary-key `Get()` — especially a guarded `if not Rec.Get(...) then exit;` — as a cheap, transaction-cached read. Do not recommend a manual `Dictionary` cache around per-row primary-key `Get` calls. Reserve N+1 concerns for genuinely repeated non-keyed queries (`FindSet`/`FindFirst` with filters, `Count`) that re-hit the database each iteration.
-
-## Anti Pattern
-
-Reporting repeated primary-key `Get` calls (such as `Bin.Get` and `BinType.Get`) inside a per-row helper as a performance defect, or recommending they be cached in a `Dictionary`. The reads are already cached by the server within the transaction, and per-row keys often differ so the calls cannot be hoisted.

microsoft/knowledge/style/tooltip-required-on-page-fields.md

@@ -15,11 +15,9 @@ CodeCop AA0218 requires a non-empty `ToolTip` property on every field control on
 
 Acceptable exceptions: table fields inside `Upgrade`, `Migration`, `HybridBC14`, `HybridSL`, and `HybridGP` codeunits and tables are allowed to omit the tooltip — those types are not surfaced to users.
 
-AA0218 is a compiler analyzer, but its severity is configured per app in the ruleset and is frequently downgraded to `info`/`None` or disabled entirely. PR review therefore cannot assume the compiler will surface the gap: it is the last line of defence for a missing tooltip and should flag it independently. The one case review must *not* flag is a bound field that inherits a `ToolTip` from its source table field — see `bound-page-field-inherits-source-field-tooltip`.
-
 ## Best Practice
 
-Every field control on a regular page carries `ToolTip = 'Specifies …';` (or a clear alternative phrasing). Compose the text in the form "what this value shows" rather than "what the user does with it". In review, raise a `medium`-severity finding for a field that has neither an inline nor an inherited tooltip, independently of whether AA0218 is active in the app's ruleset.
+Every field control on a regular page carries `ToolTip = 'Specifies …';` (or a clear alternative phrasing). Compose the text in the form "what this value shows" rather than "what the user does with it".
 
 See sample: `tooltip-required-on-page-fields.good.al`.
 

microsoft/knowledge/ui/bound-page-field-inherits-source-field-tooltip.md

@@ -13,12 +13,12 @@ application-area: [all]
 
 A page field bound to a table field inherits the source field's `ToolTip` at runtime: the control shows the table field's `ToolTip` even when the page control declares none of its own. A page field without an inline `ToolTip` is therefore not, by itself, a missing-tooltip defect — the text may be supplied by the bound source field.
 
-The genuinely-missing case is different: a bound field whose source table field *also* carries no `ToolTip`, or an unbound control, has no text to inherit and is a real accessibility gap. The compiler analyzer AA0218 detects this mechanically, but its severity is set by each app's ruleset and is routinely downgraded or disabled — so it cannot be relied on as the only net. PR review is the last line of defence and should raise this case independently.
+The genuinely-missing case — a bound field whose source table field also carries no `ToolTip`, or an unbound control that needs one — is already reported by the compiler analyzer AA0218, which BCQuality calibrates to `info`. That analyzer, not an agent finding, owns the missing-tooltip signal.
 
 ## Best Practice
 
-Do not raise a missing-`ToolTip` finding for a bound page field whose source table field supplies a `ToolTip`; assume the control inherits it. Do raise a `medium`-severity finding when the field has no inline `ToolTip` **and** no inherited one — that is, a bound field whose source field is also tooltip-less, or an unbound control — rather than assuming AA0218 will catch it downstream.
+Do not raise a missing-`ToolTip` finding for a page field that has a source-table binding; assume the source field supplies the tooltip. Reserve tooltip findings for the cases the dedicated tooltip rules define, and let analyzer AA0218 carry the mechanically-detectable missing-tooltip case at its calibrated severity.
 
 ## Anti Pattern
 
-Two opposite failures: (1) flagging every page field that has no inline `ToolTip` as a violation, ignoring that a bound field inherits its source field's tooltip; and (2) staying silent on a field that has neither an inline nor an inherited tooltip on the assumption that the compiler's AA0218 will report it — a ruleset that downgrades or disables AA0218 then lets a genuine gap ship unflagged.
+Flagging every page field that has no inline `ToolTip` property as an accessibility violation, ignoring that a bound field inherits its source field's tooltip and that AA0218 already covers the truly-missing case.

microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.good.al

@@ -1,38 +0,0 @@
-page 50210 "UI Sample Caption Case"
-{
-    PageType = List;
-    ApplicationArea = All;
-    SourceTable = "Sales Line";
-
-    layout
-    {
-        area(Content)
-        {
-            repeater(Lines)
-            {
-                field("Document No."; Rec."Document No.")
-                {
-                    ToolTip = 'Specifies the document number.';
-                }
-            }
-        }
-    }
-
-    actions
-    {
-        area(Processing)
-        {
-            action(ShowSourceDocument)
-            {
-                Caption = 'Show source document';
-                Image = ViewSourceDocumentLine;
-                ToolTip = 'Open the related source document.';
-
-                trigger OnAction()
-                begin
-                    Message('%1', Rec."Document No.");
-                end;
-            }
-        }
-    }
-}

microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.md

@@ -1,26 +0,0 @@
----
-bc-version: [all]
-domain: ui
-keywords: [caption, capitalization, sentence-case, title-case, action, noun-phrase, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# Sentence-phrase captions use sentence case, not title case
-
-## Description
-
-Business Central caption capitalization depends on whether the caption reads as a **noun phrase** or a **sentence/verb phrase**. Following the Microsoft writing-style guideline, a caption that reads as an imperative sentence — most action captions, such as `'Show source document'`, `'Post and print'`, or `'Copy from last inspection'` — uses **sentence case**: only the first word and any proper nouns are capitalized. Title case (`'Show Source Document'`) is the older convention and is not required for these captions.
-
-Noun-phrase captions (object names, field labels such as `'Source Document No.'`) follow their own capitalization; that is a separate case and is not what this article covers. Reviewers sometimes see a lower-cased word in an action caption (`'Show source document'`) and flag it as inconsistent title case, but a sentence-phrase action caption is correct as written.
-
-## Best Practice
-
-For an action `Caption` that reads as a sentence or verb phrase, capitalize only the first word and proper nouns (sentence case). Do not require every significant word to be capitalized. Before flagging a caption as "should be title case", confirm it is a noun phrase; leave imperative/sentence-phrase action captions in sentence case.
-
-See sample: `caption-capitalization-noun-phrase-vs-sentence-phrase.good.al`.
-
-## Anti Pattern
-
-Reporting a sentence-case action caption such as `'Show source document'` as a style defect and recommending title case (`'Show Source Document'`), or calling it inconsistent with BC conventions. Sentence case is the current guideline for sentence-phrase captions.

microsoft/knowledge/upgrade/obsoletereason-need-not-restate-removal-version.md

@@ -1,26 +0,0 @@
----
-bc-version: [all]
-domain: upgrade
-keywords: [obsolete-reason, obsolete-tag, deprecation, version, metadata, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# ObsoleteReason need not restate the removal version; ObsoleteTag carries it
-
-## Description
-
-An obsoleted object, field, key, enum, or enum value carries both `ObsoleteReason` and `ObsoleteTag`, and the two properties have different jobs. `ObsoleteReason` is free text that explains why the element is obsolete and what replaces it. `ObsoleteTag` identifies when it became obsolete — typically the version, release, or work item that introduced the obsoletion. The version traceability lives in `ObsoleteTag`; there is no requirement that `ObsoleteReason` also name the removal version or repeat what the tag already records. A reason that omits a version number is complete as long as it explains the deprecation and points to a replacement, provided `ObsoleteTag` pins the version.
-
-## Best Practice
-
-When `ObsoleteTag` already carries the version or tracking reference, do not flag `ObsoleteReason` for not mentioning a version or removal release. Judge `ObsoleteReason` on whether it explains the deprecation and names a replacement, and judge version traceability on `ObsoleteTag` instead.
-
-## Anti Pattern
-
-Flagging an `ObsoleteReason` as vague, incomplete, or missing a version reference solely because it does not restate the removal version, when `ObsoleteTag` already records that version. Requiring the reason to duplicate the tag's version is not a real convention.
-
-## See also
-
-- `obsoletion-requires-reason-and-tag.md` — both properties are required; the reason names the replacement and the tag identifies when the element became obsolete.

skills/bcquality-al-review/SKILL.md

@@ -24,8 +24,8 @@ Do **not** use this skill to *generate* AL code — it only reviews.
 
 ## Plugin root
 
-Resolve `PLUGIN_ROOT` to the directory that contains this plugin's root
-`plugin.json`. This skill lives at
+Resolve `PLUGIN_ROOT` to the directory that contains this plugin's
+`.claude-plugin/plugin.json`. This skill lives at
 `PLUGIN_ROOT/skills/bcquality-al-review/SKILL.md`, so `PLUGIN_ROOT` is two levels up
 from this file. All paths below are relative to `PLUGIN_ROOT`. If the host exposes a
 plugin-root environment variable, prefer it.
@@ -93,8 +93,7 @@ caller can log the reason.
   `enabled-layers` (`BCQUALITY_ENABLED_LAYERS`) — the denied layers' files still exist on
   disk. Treat `enabled-layers` as a selection filter, not a hard security boundary. A
   future revision could add a genuine deny mechanism (e.g. pruning the installed tree).
-- **Manifest location.** This plugin's manifest is the root `plugin.json`, which both
+- **Manifest location.** This plugin uses `.claude-plugin/plugin.json`, which both
   Claude Code and Copilot CLI accept (verified with Copilot CLI: `plugin install`
-  reports the bridge skill loaded). A `.claude-plugin/marketplace.json` alongside it
-  carries the marketplace entry. Claude Code also reads `.claude-plugin/plugin.json`; if
-  a future host only reads that form, dual-home the manifest there.
+  reports the bridge skill loaded). Copilot CLI also accepts a root `plugin.json`; if a
+  future host only reads the root form, dual-home the manifest.