Add events knowledge domain and review leaf skill

Add a new `events` knowledge domain covering AL events & subscribers,
wired into the AL review pipeline.

- 3 atomic articles (+ .good.al/.bad.al samples) under
  microsoft/knowledge/events/: the IsHandled override pattern, thin
  OnBefore/OnAfter integration-event publishers, and static vs manual
  subscribers.
- New leaf skill microsoft/skills/review/al-events-review.md sourcing the
  events domain.
- Wired into microsoft/skills/review/al-code-review.md (sub-skills + Source
  + description) and README.md (leaf-skill count + domain list).

AL event syntax verified against Microsoft Learn. Samples are
demonstration-only (not compiled by CI). Additive change; no contract change.

Part of #34.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

microsoft/knowledge/events/choose-static-vs-manual-subscribers-deliberately.bad.al

@@ -0,0 +1,53 @@
+// Demonstration-only AL. Not compiled by CI; illustrates the article.
+codeunit 50232 "Order Event Pub Bad Sample"
+{
+    procedure ReleaseOrder(OrderNo: Code[20])
+    begin
+        OnAfterReleaseOrder(OrderNo);
+    end;
+
+    [IntegrationEvent(false, false)]
+    local procedure OnAfterReleaseOrder(OrderNo: Code[20])
+    begin
+    end;
+}
+
+// Anti-pattern 1: a static subscriber drives an always-on side effect that
+// should be scoped. Every release now emails the customer, in every session
+// and every automated test, with no way to switch it off.
+codeunit 50233 "Always Email Sub Bad Sample"
+{
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Order Event Pub Bad Sample", 'OnAfterReleaseOrder', '', false, false)]
+    local procedure SendEmailOnRelease(OrderNo: Code[20])
+    begin
+        // Send a confirmation email unconditionally on every release.
+    end;
+}
+
+codeunit 50234 "Scoped Sub Bad Sample"
+{
+    EventSubscriberInstance = Manual;
+
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Order Event Pub Bad Sample", 'OnAfterReleaseOrder', '', false, false)]
+    local procedure OverrideRelease(OrderNo: Code[20])
+    begin
+        // Scoped behaviour intended only for a specific flow.
+    end;
+}
+
+// Anti-pattern 2: a manual subscriber is bound and never unbound. Because the
+// instance is held on a SingleInstance global, the binding lives for the whole
+// session, so later unrelated releases keep hitting the scoped subscriber.
+codeunit 50235 "Leaky Binder Bad Sample"
+{
+    SingleInstance = true;
+
+    var
+        Scoped: Codeunit "Scoped Sub Bad Sample";
+
+    procedure ActivateOverride()
+    begin
+        BindSubscription(Scoped);
+        // Missing: a matching UnbindSubscription(Scoped) when the scope ends.
+    end;
+}

microsoft/knowledge/events/choose-static-vs-manual-subscribers-deliberately.good.al

@@ -0,0 +1,52 @@
+// Demonstration-only AL. Not compiled by CI; illustrates the article.
+codeunit 50228 "Item Post Pub Good Sample"
+{
+    procedure PostItemLine(ItemNo: Code[20]; Qty: Decimal)
+    begin
+        // ... post the line ...
+        OnAfterPostItemLine(ItemNo, Qty);
+    end;
+
+    [IntegrationEvent(false, false)]
+    local procedure OnAfterPostItemLine(ItemNo: Code[20]; Qty: Decimal)
+    begin
+    end;
+}
+
+codeunit 50229 "Item Post Audit Good Sample"
+{
+    // Always-on behaviour belongs in a static subscriber (the default).
+    EventSubscriberInstance = StaticAutomatic;
+
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Item Post Pub Good Sample", 'OnAfterPostItemLine', '', false, false)]
+    local procedure LogPostedLine(ItemNo: Code[20]; Qty: Decimal)
+    begin
+        // Audit every posted line, unconditionally.
+    end;
+}
+
+codeunit 50230 "Item Post Stub Good Sample"
+{
+    // Scoped/temporary behaviour belongs in a manual subscriber.
+    EventSubscriberInstance = Manual;
+
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Item Post Pub Good Sample", 'OnAfterPostItemLine', '', false, false)]
+    local procedure CaptureForTest(ItemNo: Code[20]; Qty: Decimal)
+    begin
+        // Record the call so a single test can assert on it.
+    end;
+}
+
+codeunit 50231 "Item Post Test Good Sample"
+{
+    procedure VerifyPostingRaisesEvent()
+    var
+        Publisher: Codeunit "Item Post Pub Good Sample";
+        Stub: Codeunit "Item Post Stub Good Sample";
+    begin
+        // Activate the scoped subscriber only for the duration of the test.
+        BindSubscription(Stub);
+        Publisher.PostItemLine('1000', 5);
+        UnbindSubscription(Stub);
+    end;
+}

microsoft/knowledge/events/choose-static-vs-manual-subscribers-deliberately.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: events
+keywords: [event-subscriber, static-subscriber, manual-subscriber, bindsubscription, unbindsubscription, eventsubscriberinstance, scoped-binding]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Choose static vs manual subscribers deliberately and bind manual ones with BindSubscription
+
+## Description
+
+An `[EventSubscriber]` codeunit is static by default (`EventSubscriberInstance = StaticAutomatic`): it is always bound, so it fires for every raise of the event in every session. That is correct for always-on behaviour such as auditing, but wrong for behaviour that must be scoped — test isolation, a one-off migration, or a conditional override — because a static subscriber cannot be switched off. For scoped behaviour, set `EventSubscriberInstance = Manual` and activate the codeunit only while needed with `BindSubscription`, releasing it with `UnbindSubscription`. LLMs are largely unaware the manual model exists and default everything to static, producing always-on side effects that leak across unrelated operations and tests.
+
+## Best Practice
+
+Use a static subscriber for behaviour that genuinely applies all the time. For anything scoped, mark the codeunit `EventSubscriberInstance = Manual`, call `BindSubscription(SubscriberInstance)` at the start of the scope and `UnbindSubscription(SubscriberInstance)` at the end. A manual subscriber held only in a local variable unbinds automatically when that variable leaves scope, which suits test setup/teardown; a binding you intend to outlive a single call must be unbound explicitly. Keep subscriber methods `local` per CodeCop AA0207.
+
+See sample: `choose-static-vs-manual-subscribers-deliberately.good.al`.
+
+## Anti Pattern
+
+Two shapes. First, a static subscriber used for behaviour that should be scoped — an always-on side effect (sending mail, writing extra records) that now fires for every event in every session and test with no way to disable it. Second, a manual subscriber that is bound with `BindSubscription` and never unbound: when the instance is held beyond the intended scope (for example on a `SingleInstance` codeunit), the binding leaks for the whole session and later unrelated operations keep hitting it. Detection: scoped side effects on a static subscriber, or a `BindSubscription` call with no matching `UnbindSubscription` and no scope that releases the instance.
+
+See sample: `choose-static-vs-manual-subscribers-deliberately.bad.al`.

microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.bad.al

@@ -0,0 +1,36 @@
+// Demonstration-only AL. Not compiled by CI; illustrates the article.
+table 50226 "Reservation Entry Bad Sample"
+{
+    fields
+    {
+        field(1; "Entry No."; Integer) { }
+        field(2; "Item No."; Code[20]) { }
+        field(3; Quantity; Decimal) { }
+        field(4; Reserved; Boolean) { }
+    }
+    keys
+    {
+        key(PK; "Entry No.") { Clustered = true; }
+    }
+}
+
+codeunit 50227 "Reservation Post Bad Sample"
+{
+    procedure Reserve(var ReservationEntry: Record "Reservation Entry Bad Sample")
+    begin
+        // Anti-pattern: the operation exposes no OnBefore/OnAfter seam, and the
+        // logic that should be the routine's own work lives in the event body
+        // below instead. Partners must overwrite this routine to change it.
+        OnReserve(ReservationEntry);
+    end;
+
+    // Anti-pattern: business logic inside an integration-event publisher. A
+    // publisher must be a thin, empty hook; logic placed here runs on every
+    // raise and cannot be overridden, which defeats the event entirely.
+    [IntegrationEvent(false, false)]
+    local procedure OnReserve(var ReservationEntry: Record "Reservation Entry Bad Sample")
+    begin
+        ReservationEntry.Reserved := true;
+        ReservationEntry.Modify(true);
+    end;
+}

microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.good.al

@@ -0,0 +1,43 @@
+// Demonstration-only AL. Not compiled by CI; illustrates the article.
+table 50224 "Reservation Entry Sample"
+{
+    fields
+    {
+        field(1; "Entry No."; Integer) { }
+        field(2; "Item No."; Code[20]) { }
+        field(3; Quantity; Decimal) { }
+        field(4; Reserved; Boolean) { }
+    }
+    keys
+    {
+        key(PK; "Entry No.") { Clustered = true; }
+    }
+}
+
+codeunit 50225 "Reservation Post Good Sample"
+{
+    procedure Reserve(var ReservationEntry: Record "Reservation Entry Sample")
+    var
+        IsHandled: Boolean;
+    begin
+        OnBeforeReserve(ReservationEntry, IsHandled);
+        if IsHandled then
+            exit;
+
+        ReservationEntry.Reserved := true;
+        ReservationEntry.Modify(true);
+
+        OnAfterReserve(ReservationEntry);
+    end;
+
+    // Thin publishers: empty bodies, the calling routine owns the logic.
+    [IntegrationEvent(false, false)]
+    local procedure OnBeforeReserve(var ReservationEntry: Record "Reservation Entry Sample"; var IsHandled: Boolean)
+    begin
+    end;
+
+    [IntegrationEvent(false, false)]
+    local procedure OnAfterReserve(var ReservationEntry: Record "Reservation Entry Sample")
+    begin
+    end;
+}

microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: events
+keywords: [integration-event, onbefore, onafter, extension-point, thin-publisher, publisher-body, extensibility]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Publish thin OnBefore/OnAfter integration events to expose extension points
+
+## Description
+
+A key operation — a posting, release, or validation routine — becomes a hard wall for partners when it ships no integration events: the only way to change it is to overwrite or duplicate the base code. The Business Central remedy is to raise thin `OnBeforeX`/`OnAfterX` integration events at the operation's boundaries, passing `var Rec` and the relevant parameters so subscribers have what they need. An equally common defect is the inverse: putting business logic *inside* the publisher method body. An event publisher is a hook, not a procedure — its body must be empty, and the platform even forbids variables, return values, and code other than comments in it. LLMs both omit the extension points and, when they do add an event, wrongly fill its body with logic.
+
+## Best Practice
+
+Wrap the operation's core with events: raise `OnBeforeX(var Rec, var IsHandled)` before the default work and `OnAfterX(var Rec)` once it succeeds, at the natural boundaries of the routine. Declare each publisher `[IntegrationEvent(false, false)] local procedure` with an empty body and let the calling routine — never the publisher — own the logic. Pass records by `var` so subscribers can read and adjust them, and include the parameters a subscriber would need to act. This gives partners a stable seam without touching base code.
+
+See sample: `publish-thin-onbefore-onafter-integration-events.good.al`.
+
+## Anti Pattern
+
+Business logic placed inside an `[IntegrationEvent]` publisher method, so the "event" actually mutates state every time it is raised — defeating the hook and surprising every reader — or a core operation that exposes no extension points at all, forcing partners to overwrite or duplicate it. Detection: an `[IntegrationEvent]`/`[BusinessEvent]` method whose body contains statements rather than being empty, or a posting/validation routine with no surrounding `OnBefore`/`OnAfter` publishers.
+
+See sample: `publish-thin-onbefore-onafter-integration-events.bad.al`.

microsoft/knowledge/events/use-ishandled-to-make-base-behaviour-overridable.bad.al

@@ -0,0 +1,38 @@
+// Demonstration-only AL. Not compiled by CI; illustrates the article.
+
+// Anti-pattern 1: no OnBefore/IsHandled hook. A partner cannot replace this
+// rule without overwriting base code, so the behaviour is not extensible.
+codeunit 50222 "Shipping Charge NoHook Bad"
+{
+    procedure CalculateShippingCharge(OrderAmount: Decimal) Charge: Decimal
+    begin
+        if OrderAmount >= 1000 then
+            Charge := 0
+        else
+            Charge := 49;
+    end;
+}
+
+// Anti-pattern 2: the hook exists but the 'if IsHandled then exit;' guard is
+// missing, so the default logic still runs after a subscriber handled the call.
+codeunit 50223 "Shipping Charge Guard Bad"
+{
+    procedure CalculateShippingCharge(OrderAmount: Decimal) Charge: Decimal
+    var
+        IsHandled: Boolean;
+    begin
+        OnBeforeCalculateShippingCharge(OrderAmount, Charge, IsHandled);
+
+        // Bug: no 'if IsHandled then exit;' here. Even when a subscriber set
+        // Charge and IsHandled := true, the default below overwrites the result.
+        if OrderAmount >= 1000 then
+            Charge := 0
+        else
+            Charge := 49;
+    end;
+
+    [IntegrationEvent(false, false)]
+    local procedure OnBeforeCalculateShippingCharge(OrderAmount: Decimal; var Charge: Decimal; var IsHandled: Boolean)
+    begin
+    end;
+}

microsoft/knowledge/events/use-ishandled-to-make-base-behaviour-overridable.good.al

@@ -0,0 +1,37 @@
+// Demonstration-only AL. Not compiled by CI; illustrates the article.
+codeunit 50220 "Shipping Charge Good Sample"
+{
+    procedure CalculateShippingCharge(OrderAmount: Decimal) Charge: Decimal
+    var
+        IsHandled: Boolean;
+    begin
+        // Give extensions a sanctioned seam to replace the calculation, then
+        // skip the default logic when a subscriber has handled it.
+        OnBeforeCalculateShippingCharge(OrderAmount, Charge, IsHandled);
+        if IsHandled then
+            exit(Charge);
+
+        if OrderAmount >= 1000 then
+            Charge := 0
+        else
+            Charge := 49;
+    end;
+
+    [IntegrationEvent(false, false)]
+    local procedure OnBeforeCalculateShippingCharge(OrderAmount: Decimal; var Charge: Decimal; var IsHandled: Boolean)
+    begin
+    end;
+}
+
+codeunit 50221 "Shipping Charge Sub Good Sample"
+{
+    // A partner replaces the flat rate with a contract-specific rule.
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Shipping Charge Good Sample", 'OnBeforeCalculateShippingCharge', '', false, false)]
+    local procedure ApplyContractRate(OrderAmount: Decimal; var Charge: Decimal; var IsHandled: Boolean)
+    begin
+        if IsHandled then
+            exit;
+        Charge := OrderAmount * 0.02;
+        IsHandled := true;
+    end;
+}

microsoft/knowledge/events/use-ishandled-to-make-base-behaviour-overridable.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: events
+keywords: [ishandled, overridable, onbefore, integration-event, extensibility, event-override, subscriber-hook]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Use the IsHandled pattern to make base behaviour overridable
+
+## Description
+
+AL has no method overriding, so a `procedure` that runs its body unconditionally cannot be replaced by an extension without editing base code. The established Business Central seam for substituting default behaviour is the `IsHandled` pattern: the routine raises an `OnBefore…` integration event carrying a `var IsHandled: Boolean`, then exits early when a subscriber has set it. This hands a partner a sanctioned hook to replace the logic instead of overwriting the routine. LLMs trained on languages with inheritance emit routines whose logic always runs and expose no `OnBefore`/`IsHandled` seam, so the behaviour silently cannot be overridden.
+
+## Best Practice
+
+Raise `OnBeforeX(…, IsHandled)` as the first step of the routine and guard with `if IsHandled then exit;` before any default logic runs. Declare the publisher `[IntegrationEvent(false, false)] local procedure OnBeforeX(…; var IsHandled: Boolean)` with an empty body, and keep `IsHandled` a `var` parameter so a subscriber can write to it. A subscriber that replaces the behaviour does its work and sets `IsHandled := true`; one that only augments leaves it untouched and guards with `if IsHandled then exit;` itself. Reserve the override hook for cases where a partner genuinely needs to replace logic — when the goal is only to react, a positive `OnAfter` event is the better seam.
+
+See sample: `use-ishandled-to-make-base-behaviour-overridable.good.al`.
+
+## Anti Pattern
+
+Two shapes. First, a routine whose default logic always runs because there is no `OnBefore…`/`IsHandled` hook at all — extensions cannot change it without overwriting base code. Second, a routine that raises `OnBeforeX(IsHandled)` but omits the `if IsHandled then exit;` guard, so the default logic still executes after a subscriber set `IsHandled := true`, duplicating work and side effects. Detection: an `OnBefore` publisher with a `var IsHandled: Boolean` parameter whose caller never tests `IsHandled`, or a public routine doing non-trivial work with no overridable seam.
+
+See sample: `use-ishandled-to-make-base-behaviour-overridable.bad.al`.