.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",
+      "version": "0.1.0"
-      "skills": [
-        "./skills/bcquality-al-review/"
-      ]
     }
   ]
 }

.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/"
-  ]
 }

README.md

@@ -18,8 +18,6 @@ Poor fit: "Use HTTPS instead of HTTP." "Don't hardcode secrets." "Keep transacti
 
 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.
@@ -149,7 +147,6 @@ Contributions are welcome. Before submitting a PR:
 1. Read the knowledge file format above — frontmatter and sections are validated by CI.
 2. Keep files atomic: one concern per file, under 100 lines.
 3. Target your contribution to the right layer — most community contributions go in `/community/knowledge/`.
-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.
 

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/breaking-changes/unreleased-symbol-change-is-not-a-breaking-change.md

@@ -1,24 +0,0 @@
----
-bc-version: [all]
-domain: breaking-changes
-keywords: [released-baseline, unreleased, rename, renumber, obsolete, api-stability, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# Changing an unreleased symbol is not a breaking change
-
-## Description
-
-Breaking-change rules protect contracts that have already shipped to customers or are exposed to external extensions. A symbol — an object, field, key, enum value, or procedure — that is new in this app, was introduced and then changed within the same still-unreleased development cycle, or belongs to an app that has no released version yet, can be renamed, renumbered, or removed freely. There is no shipped contract to break, so the change is not a breaking change.
-
-Release status is established from the diff, the app's `app.json` version, or a released baseline. An app whose `app.json` version has no corresponding released baseline (for example a `1.0.0.0` app that has never shipped) has no protected surface.
-
-## Best Practice
-
-Before treating a rename, renumber, or removal as breaking, establish that the affected symbol was present in a released baseline. Do not flag changes to symbols that are new in the current unreleased cycle or that belong to an app with no released version. When release status cannot be established from the diff, `app.json`, or a released baseline, omit the finding rather than assert a break.
-
-## Anti Pattern
-
-Reporting a breaking change for a rename, renumber, or removal without confirming the symbol shipped in a released version — for example flagging a break on an app whose `app.json` version has no released baseline.

microsoft/knowledge/error-handling/page-boolean-triggers-default-to-true.md

@@ -1,24 +0,0 @@
----
-bc-version: [all]
-domain: error-handling
-keywords: [oninsertrecord, onmodifyrecord, ondeleterecord, onquerypage, boolean-trigger, exit, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# Page record triggers return true by default; a missing exit(true) does not block the operation
-
-## Description
-
-The Boolean page record triggers `OnInsertRecord`, `OnModifyRecord`, `OnDeleteRecord`, and `OnQueryClosePage` return `true` by default. When the trigger body omits an explicit return value, the platform treats the result as `true` and the operation proceeds. Only an explicit `exit(false)` — or a reachable code path that returns `false` — cancels the insert, modify, delete, or page close.
-
-This is a defined exception to the ordinary Boolean method rule, where the default return is `false`. Reviewers unfamiliar with the exception sometimes read a page record trigger that has no `exit(true)` and conclude the operation is blocked; it is not.
-
-## Best Practice
-
-Do not claim that a missing `exit(true)` blocks or prevents an insert, modify, or delete, and do not recommend adding `exit(true)` "to let the operation proceed" — that is already the default. Evaluate these triggers only for an explicit or reachable `exit(false)`/false-returning path that would cancel the operation unintentionally.
-
-## Anti Pattern
-
-Flagging `OnInsertRecord`, `OnModifyRecord`, `OnDeleteRecord`, or `OnQueryClosePage` as defective because it "does not return `true`", or asserting that inserts/modifies/deletes will silently fail without an explicit `exit(true)`. The default return already permits the operation.

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/calcfields-in-both-getrecord-triggers-is-not-redundant.md

@@ -1,22 +0,0 @@
----
-bc-version: [all]
-domain: performance
-keywords: [calcfields, onaftergetrecord, onaftergetcurrrecord, page-lifecycle, flowfield, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# CalcFields in both OnAfterGetRecord and OnAfterGetCurrRecord is not redundant
-
-## Description
-
-`OnAfterGetRecord` fires once per row as the page loads records into the view; `OnAfterGetCurrRecord` fires when a record becomes the active/current record. Calling `CalcFields` in both triggers is not duplicate or redundant work: the two triggers run at different points in the page lifecycle and serve different purposes — populating FlowFields for every displayed row versus refreshing them for the record the user has selected. The same `CalcFields` call appearing in both places is an intentional pattern, not copy-paste waste.
-
-## Best Practice
-
-Do not flag `CalcFields` appearing in both `OnAfterGetRecord` and `OnAfterGetCurrRecord` as duplicate, redundant, or removable. Treat each trigger's `CalcFields` on its own lifecycle merits.
-
-## Anti Pattern
-
-Recommending that a developer delete one of the two `CalcFields` calls because "the field is already calculated in the other trigger". The genuine per-row FlowField cost is addressed by the separate guidance on FlowField calculation in loops and on hidden FlowFields; it is not addressed by removing a lifecycle-correct `CalcFields`.

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/page-effective-filter-may-live-outside-the-diff.md

@@ -1,22 +0,0 @@
----
-bc-version: [all]
-domain: performance
-keywords: [filter, drilldown, lookup, sourcetableview, tablerelation, setrange, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# A page or lookup's effective filter may be defined outside the changed hunk
-
-## Description
-
-The effective filter on a drill-down, lookup, or list result set is frequently defined outside any single changed hunk — on the table via a `SourceTableView` property or a `TableRelation`, or through `SetRange`/`SetFilter` calls in unchanged code that runs before the result is shown. The absence of a filter within the changed lines of a diff is therefore not evidence that the result set is unfiltered or that it will load an entire table.
-
-## Best Practice
-
-Do not assert that a drill-down, lookup, or list is "unfiltered" based only on the changed hunk. Confirm the effective filter by checking the page's `SourceTableView`, the field's `TableRelation`, and any `SetRange`/`SetFilter` in the surrounding (possibly unchanged) code before raising a finding about an unbounded result set.
-
-## Anti Pattern
-
-Concluding that a lookup or drill-down loads an unfiltered, full-table result set solely because no `SetRange`/`SetFilter` appears in the changed lines, when the filter is defined on the table, in a `TableRelation`, or in unchanged setup code.

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

@@ -1,24 +0,0 @@
----
-bc-version: [all]
-domain: ui
-keywords: [tooltip, page-field, source-field, inheritance, aa0218, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# A page field bound to a table field inherits that field's ToolTip
-
-## Description
-
-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.
-
-## 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.
-
-## 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.

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.

microsoft/knowledge/upgrade/unreleased-schema-change-needs-no-upgrade-path.md

@@ -1,24 +0,0 @@
----
-bc-version: [all]
-domain: upgrade
-keywords: [released-baseline, unreleased, schema, migration, obsolete, data-loss, false-positive]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# Unreleased schema changes need no upgrade or migration path
-
-## Description
-
-Upgrade and migration findings protect data and schema that have already shipped to customers. A schema element — a table, field, key, or enum — that is new in this app, or was added and then changed within the same still-unreleased development cycle, needs no upgrade code or migration path: no customer has data in it yet, so there is nothing to preserve or migrate. Such a change is not an obsoletion, data-loss, or breaking-migration defect.
-
-Release status is established from the diff, the app's `app.json` version, or a released baseline. A schema element with no released baseline has no persisted customer data to protect.
-
-## Best Practice
-
-Before asserting an obsoletion, data-loss, or breaking-migration defect, establish that the affected table, field, key, or enum existed in a released version. Do not require upgrade or migration code for schema that never shipped. When release status cannot be established from the diff, `app.json`, or a released baseline, omit the finding rather than demand a migration path.
-
-## Anti Pattern
-
-Demanding an upgrade codeunit, migration path, or data-preservation step, or flagging data loss, for a table, field, key, or enum that is new in the current unreleased cycle and has no released baseline.

microsoft/skills/review/al-privacy-review.md

@@ -68,7 +68,7 @@ Set `confidence` to:
 - `medium` when detection relies on heuristics or when any frontmatter dimension was `unknown`.
 - `low` when the finding is an advisory derived only from applicability.
 
-This leaf emits only knowledge-backed privacy findings. Do NOT emit reference-less `agent:` findings in this domain: online evaluation shows the privacy agent-finding channel yields almost no accepted findings and a high volume of dismissed noise, so a privacy concern that no worklist knowledge file covers is omitted here rather than emitted with `references: []`. When you spot a material privacy defect no article covers, the durable fix is to add a knowledge article in BCQuality (per the online-eval self-improvement loop) so this leaf can cite it — not a one-off reference-less finding. Before treating a candidate as uncovered, check the worklist for a knowledge file that matches it; if one exists, emit it as a knowledge-backed finding. See `skills/do.md` for the full contract.
+After evaluating each worklist entry, also consider whether the diff exhibits a privacy defect the agent recognises from its general AL knowledge that no knowledge file in the worklist covers. Such candidates are agent findings within this skill's domain — emit them with `references: []`, an `id` slug prefixed with `agent:`, `confidence` capped at `medium`, `severity` capped at `minor` (agent findings are advisory and non-gating), and a `message` that is self-contained (describing both the issue and a concrete recommendation, since there is no knowledge-file footer for the consumer to fall back on). Hold every candidate to the precision bar in `skills/do.md` (*Agent findings*): emit only a concrete, material privacy defect a knowledgeable BC reviewer would agree is wrong — steelman it first and drop anything stylistic, speculative, dependent on code outside the diff, or merely a valid alternative; when in doubt, omit. The scope is strictly privacy; defects outside this domain belong to other leaves and MUST NOT be emitted here. Before emitting, check the worklist for a knowledge file that matches the candidate — if one exists, upgrade the candidate to a knowledge-backed finding instead. See `skills/do.md` for the full contract.
 
 For every emitted finding, decide whether the fix is mechanical. A fix is mechanical when it is small, local, and unambiguous from the diff context (for example: delete unreachable lines; replace `Count() > 0` with `not IsEmpty()`; move a local `Label` to object scope; add a missing `ToolTip`, `OptionCaption`, or `DataClassification`; replace a string-concatenated `Error` with a Label-backed call; change an over-broad permission token; or add an obvious `else`/guard branch). For mechanical findings, emit `findings[].suggested-code` with the literal replacement for the source lines indicated by `location`. The payload must be a verbatim replacement — no diff markers, no fences, no commentary — that the consumer can render as a one-click suggestion. When a `.good.al` companion exists and the diff context matches the `.bad.al` shape, adapt the `.good.al` replacement into `suggested-code`.
 

microsoft/skills/review/al-style-review.md

@@ -60,7 +60,7 @@ When the post-conflict worklist is empty because no applicable style knowledge e
 
 For each worklist entry, evaluate the diff against the file's `## Best Practice` and `## Anti Pattern` sections. Style findings rarely reach `blocker` — reserve it for cases where the knowledge file documents a platform-level requirement (for example, API page property constraints the OData runtime rejects). Most style findings are `minor` or `info`; egregious misuse (`Error` with pre-built Text losing translation and telemetry classification) may reach `major`.
 
-Severity calibration — a formal analyzer already flags the mechanical presence/naming conventions (the `this` keyword AA0248, approved label suffixes AA0074, variable-declaration order by type AA0021, a missing `ToolTip`, required parentheses). On those, BCQuality's value is the *explanation* of why the rule exists, not a second gate; emit them at `info` so a consumer that gates on severity does not re-flag what CodeCop/AppSourceCop already reports. Reserve `minor` for style issues with concrete downstream impact the analyzer does not catch — a `Label` declared at procedure-local instead of object scope (no analyzer enforces label scope, and mis-scoped Labels are fragile in the translation pipeline), lost translation or telemetry classification from a string-built `Error`, an `OptionCaption` that does not match its `OptionMembers`, a misleading named invocation. This keeps the domain's default output advisory and prevents analyzer-redundant noise from competing with substantive review.
+Severity calibration — a formal analyzer already flags the mechanical presence/naming conventions (the `this` keyword AA0248, approved label suffixes AA0074, a missing `ToolTip`, required parentheses). On those, BCQuality's value is the *explanation* of why the rule exists, not a second gate; emit them at `info` so a consumer that gates on severity does not re-flag what CodeCop/AppSourceCop already reports. Reserve `minor` for style issues with concrete downstream impact the analyzer does not catch — a `Label` declared at procedure-local instead of object scope (no analyzer enforces label scope, and mis-scoped Labels are fragile in the translation pipeline), lost translation or telemetry classification from a string-built `Error`, an `OptionCaption` that does not match its `OptionMembers`, a misleading named invocation. This keeps the domain's default output advisory and prevents analyzer-redundant noise from competing with substantive review.
 
 Set `confidence` to:
 

microsoft/skills/review/al-ui-review.md

@@ -61,7 +61,7 @@ Set `confidence` to:
 - `medium` when detection relies on heuristics (judging whether a caption is a noun phrase or a sentence phrase) or when any frontmatter dimension was `unknown`.
 - `low` when the finding is an advisory derived only from applicability.
 
-This leaf emits only knowledge-backed UI and accessibility findings. Do NOT emit reference-less `agent:` findings in this domain: online evaluation shows the UI/accessibility agent-finding channel yields almost no accepted findings and a high volume of dismissed noise, so a UI or accessibility concern that no worklist knowledge file covers is omitted here rather than emitted with `references: []`. When you spot a material UI or accessibility defect no article covers, the durable fix is to add a knowledge article in BCQuality (per the online-eval self-improvement loop) so this leaf can cite it — not a one-off reference-less finding. Before treating a candidate as uncovered, check the worklist for a knowledge file that matches it; if one exists, emit it as a knowledge-backed finding. See `skills/do.md` for the full contract.
+After evaluating each worklist entry, also consider whether the diff exhibits a UI defect the agent recognises from its general AL knowledge that no knowledge file in the worklist covers. Such candidates are agent findings within this skill's domain — emit them with `references: []`, an `id` slug prefixed with `agent:`, `confidence` capped at `medium`, `severity` capped at `minor` (agent findings are advisory and non-gating), and a `message` that is self-contained (describing both the issue and a concrete recommendation, since there is no knowledge-file footer for the consumer to fall back on). Hold every candidate to the precision bar in `skills/do.md` (*Agent findings*): emit only a concrete, material UI defect a knowledgeable BC reviewer would agree is wrong — steelman it first and drop anything speculative, dependent on code outside the diff, or merely a valid alternative; when in doubt, omit. The scope is strictly UI; defects outside this domain belong to other leaves and MUST NOT be emitted here. Before emitting, check the worklist for a knowledge file that matches the candidate — if one exists, upgrade the candidate to a knowledge-backed finding instead. See `skills/do.md` for the full contract.
 
 For every emitted finding, decide whether the fix is mechanical. A fix is mechanical when it is small, local, and unambiguous from the diff context (for example: delete unreachable lines; replace `Count() > 0` with `not IsEmpty()`; move a local `Label` to object scope; add a missing `ToolTip`, `OptionCaption`, or `DataClassification`; replace a string-concatenated `Error` with a Label-backed call; change an over-broad permission token; or add an obvious `else`/guard branch). For mechanical findings, emit `findings[].suggested-code` with the literal replacement for the source lines indicated by `location`. The payload must be a verbatim replacement — no diff markers, no fences, no commentary — that the consumer can render as a one-click suggestion. When a `.good.al` companion exists and the diff context matches the `.bad.al` shape, adapt the `.good.al` replacement into `suggested-code`.
 

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
+Resolve `PLUGIN_ROOT` to the directory that contains this plugin's
-`plugin.json`. This skill lives at
+`.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
+  reports the bridge skill loaded). Copilot CLI also accepts a root `plugin.json`; if a
-  carries the marketplace entry. Claude Code also reads `.claude-plugin/plugin.json`; if
+  future host only reads the root form, dual-home the manifest.
-  a future host only reads that form, dual-home the manifest there.

skills/do.md

@@ -21,19 +21,6 @@ An action skill is a single markdown file with YAML frontmatter. It lives inside
 
 Action skills do not live at the repo root. The files in `/skills/` — the three meta-skill contracts (READ, DO, WRITE) and the entry-point skill (`entry.md`, `kind: entry-point`) — are the only skills that sit outside a layer. The entry-point skill structurally follows this same four-step pattern but produces a dispatch record rather than a findings-report; see `skills/entry.md` for its contract.
 
-## Skills hold mechanics; knowledge files hold BC facts
-
-An action skill is a *finder and applier*: its prose says how to discover candidate knowledge (Source), filter it (Relevance), narrow it to the task (Worklist), and shape output (Action). Every Business-Central-specific behavioural claim a skill acts on — what a property defaults to, what a trigger does, why a given shape is or is not a defect — belongs in a knowledge file the skill cites, not in the skill prose.
-
-This includes **negative knowledge**. A false-positive guard — "pattern X is not a defect, because BC does Y" — is as much a knowledge file as a positive best practice. When an eval shows the agent over-reporting a pattern, the fix is a knowledge file documenting why the pattern is legitimate, so the skill can cite it and any leaf can reuse it — not a hard-coded exclusion buried in one skill. See `skills/write.md` (*Is this a knowledge file?*).
-
-Two rules follow for skill authors:
-
-- **Do not add a BC fact to a skill.** If you are editing a skill to change *what it flags* — adding an exclusion, encoding a platform default, teaching it that some pattern is fine — you are holding a knowledge file, not a skill edit. Author the knowledge file and let Worklist route to it.
-- **Do not restate an article's fact inline.** A Worklist cue may name the article to load and the diff shape that selects it; it must not re-assert the article's reasoning, which then drifts from the source. Cite, don't copy.
-
-The meta-skills themselves (`read.md`, `do.md`, `write.md`) are domain-agnostic templates and carry no BC-specific rule.
-
 ## Frontmatter schema
 
 ```yaml

skills/write.md

@@ -9,25 +9,6 @@ title: New Knowledge — how to author a knowledge file
 
 Anyone — human or agent — adding a knowledge file to BCQuality follows this guide. READ is the format specification; WRITE is the authoring guide. This file does not restate the schema; consult READ for field-by-field semantics.
 
-## Is this a knowledge file?
-
-Before authoring anything, confirm a knowledge file is the right artifact. BCQuality separates *mechanics* from *facts*:
-
-- **Skills** (`*/skills/**`) hold only finder/applier mechanics — how to discover, filter, worklist, and emit findings. See `skills/do.md`.
-- **Knowledge files** (`*/knowledge/**`) hold every Business-Central-specific fact a skill acts on.
-
-A new BC fact is therefore a knowledge file, never a skill edit. In particular, if you arrived here because a review agent flagged something it should not have (a false positive) or missed something it should have caught, the remedy is a knowledge file — apply the admission test in the [README](../README.md#what-belongs-here): *would a capable LLM get this wrong without the file?* If you find yourself editing a skill to stop it flagging something, stop and write a knowledge file instead.
-
-### Negative knowledge is first-class
-
-A knowledge file does not have to recommend an action. A **negative clarification** — "pattern X is *not* a defect, because BC behaves as Y" — is a first-class knowledge file, authored exactly like a positive rule:
-
-- **Description** states the BC behaviour that makes the pattern legitimate.
-- **Best Practice** tells the reviewer or agent what *not* to flag, and why.
-- **Anti Pattern** describes the false-positive report itself — the mistaken finding to suppress.
-
-For example, `microsoft/knowledge/error-handling/page-boolean-triggers-default-to-true.md` records that the Boolean page record triggers return `true` by default, so a "missing `exit(true)`" report is not a real defect. It reads as ordinary knowledge; its anti-pattern is the incorrect review comment, not the code.
-
 ## Before you start
 
 Read `skills/read.md` first. A file that does not conform to READ will be rejected. WRITE assumes READ is already understood.