From fc544f7bd5fdd6851158b2b076bdcd8f23285825 Mon Sep 17 00:00:00 2001 From: waldo1001 <12088142+waldo1001@users.noreply.github.com> Date: Tue, 15 Sep 2026 11:21:52 +0200 Subject: [PATCH] knowledge(events): serialize the setup-counter increment; promote article and wire the review skill PR #152 round 3 (JesperSchulz): - The good sample's OnAfterInsertEvent subscriber still raced on the shared "Transfer Setup Good" singleton (Get/increment/Modify); AutoIncrement only protected the request key. Added TransferSetup.LockTable() before Get() to serialize concurrent background sessions. - Promoted changecompany-runs-triggers-in-the-calling-company from community/knowledge/events/ to microsoft/knowledge/events/, and wired ChangeCompany/StartSession/RunTrigger tokens plus a targeted detection cue into microsoft/skills/review/al-events-review.md so a diff containing the anti-pattern reliably worklists this article, preserving the documented RunTrigger=false hand-off exception. Co-Authored-By: Claude Sonnet 5 --- .../changecompany-runs-triggers-in-the-calling-company.bad.al | 0 .../changecompany-runs-triggers-in-the-calling-company.good.al | 2 ++ .../changecompany-runs-triggers-in-the-calling-company.md | 0 microsoft/skills/review/al-events-review.md | 3 ++- 4 files changed, 4 insertions(+), 1 deletion(-) rename {community => microsoft}/knowledge/events/changecompany-runs-triggers-in-the-calling-company.bad.al (100%) rename {community => microsoft}/knowledge/events/changecompany-runs-triggers-in-the-calling-company.good.al (94%) rename {community => microsoft}/knowledge/events/changecompany-runs-triggers-in-the-calling-company.md (100%) diff --git a/community/knowledge/events/changecompany-runs-triggers-in-the-calling-company.bad.al b/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.bad.al similarity index 100% rename from community/knowledge/events/changecompany-runs-triggers-in-the-calling-company.bad.al rename to microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.bad.al diff --git a/community/knowledge/events/changecompany-runs-triggers-in-the-calling-company.good.al b/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.good.al similarity index 94% rename from community/knowledge/events/changecompany-runs-triggers-in-the-calling-company.good.al rename to microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.good.al index b411bd8..dd92d9d 100644 --- a/community/knowledge/events/changecompany-runs-triggers-in-the-calling-company.good.al +++ b/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.good.al @@ -75,6 +75,8 @@ codeunit 50101 "Transfer Request Count Good" var TransferSetup: Record "Transfer Setup Good"; begin + // Serializes the read-modify-write so concurrent background sessions don't lose an increment. + TransferSetup.LockTable(); TransferSetup.Get(); TransferSetup."Open Requests" += 1; TransferSetup.Modify(); diff --git a/community/knowledge/events/changecompany-runs-triggers-in-the-calling-company.md b/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.md similarity index 100% rename from community/knowledge/events/changecompany-runs-triggers-in-the-calling-company.md rename to microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.md diff --git a/microsoft/skills/review/al-events-review.md b/microsoft/skills/review/al-events-review.md index 68bcdb0..b257d3d 100644 --- a/microsoft/skills/review/al-events-review.md +++ b/microsoft/skills/review/al-events-review.md @@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed AL object names and types — especially codeunits that publish events or host event subscribers, posting/release/validation routines that should expose extension points, and test codeunits that bind subscribers. - The changed procedures and triggers, weighted toward event publisher methods, methods carrying the `[EventSubscriber(...)]` attribute, routines that raise `OnBefore`/`OnAfter` events, and any procedure that calls `BindSubscription`/`UnbindSubscription`. -- Tokens extracted from the diff that relate to events and the publish/subscribe model (`IntegrationEvent`, `BusinessEvent`, `InternalEvent`, `EventSubscriber`, `IsHandled`, `BindSubscription`, `UnbindSubscription`, `EventSubscriberInstance`, `OnBefore`, `OnAfter`, `Manual`, `IncludeSender`, `GlobalVarAccess`, `Isolated`, `local`, `internal`, `Sender`, `this`, `RecordRef`, `xRec`, `temporary`, `Temp`, `repeat`). +- Tokens extracted from the diff that relate to events and the publish/subscribe model (`IntegrationEvent`, `BusinessEvent`, `InternalEvent`, `EventSubscriber`, `IsHandled`, `BindSubscription`, `UnbindSubscription`, `EventSubscriberInstance`, `OnBefore`, `OnAfter`, `Manual`, `IncludeSender`, `GlobalVarAccess`, `Isolated`, `local`, `internal`, `Sender`, `this`, `RecordRef`, `xRec`, `temporary`, `Temp`, `repeat`, `ChangeCompany`, `StartSession`, `RunTrigger`). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. @@ -65,6 +65,7 @@ The following targeted checks map diff signals to specific `events` articles. Tr - A `RecordRef` event parameter, or a passed-through `xRec`, where a concrete typed record fits — `avoid-loosely-typed-event-parameters`. - A `var IsHandled` added to a pre-existing event rather than introduced through a new `OnBefore` publisher — `do-not-add-ishandled-to-an-existing-event`. - An `if IsHandled then exit;` whose skipped body performs posting, ledger-entry creation, number-series consumption, or integrity/permission validation — `do-not-bypass-critical-operations-with-ishandled`. +- A record variable that had `ChangeCompany()` called on it and is later used with `Insert`, `Modify`, `Delete`, or `Validate`, where the table is not owned by the extension, has triggers that read company data, or has trigger-event subscribers that do not exit on `RunTrigger = false` — `changecompany-runs-triggers-in-the-calling-company`. Do not match a read-only use after `ChangeCompany`, a write with `RunTrigger = false` into an extension-owned table whose triggers do not read company data and whose trigger-event subscribers exit on `RunTrigger = false`, or the parameterless `ChangeCompany()` reset. ## Action