Merge pull request #124 from microsoft/bcq/642303-fp-guards

Add FP guards: field relocation to tableextension + event parameter addition (bug 642303)

.claude-plugin/marketplace.json

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

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

@@ -0,0 +1,18 @@
+---
+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/events/adding-a-parameter-to-an-event-is-not-a-breaking-change.md

@@ -0,0 +1,18 @@
+---
+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.

plugin.json

@@ -5,5 +5,17 @@
   "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/"
+  ]
 }

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
-`.claude-plugin/plugin.json`. This skill lives at
+Resolve `PLUGIN_ROOT` to the directory that contains this plugin's root
+`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,7 +93,8 @@ 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 uses `.claude-plugin/plugin.json`, which both
+- **Manifest location.** This plugin's manifest is the root `plugin.json`, which both
   Claude Code and Copilot CLI accept (verified with Copilot CLI: `plugin install`
-  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.
+  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.