UI knowledge: notification recall needs a known Id; open mixed-type documents through Page Management (#214)

* Add UI knowledge: notification recall Id and Page Management document routing

Two ui articles with compiled good/bad samples:
- notification-recall-needs-known-id: a notification that the code also
  recalls needs a fixed Id (single instance; recall before re-send) or
  per-record tracking through codeunit "Notification Lifecycle Mgt."
  (SendNotification[WithAdditionalContext] / RecallNotificationsForRecord,
  HandleDelayedInsert semantics). Never-recalled notifications are exempt.
- list-page-document-routing-uses-page-management: open documents from a
  mixed-type list with PageManagement.PageRun(Rec) instead of a hand-written
  case "Document Type" / Page.Run; register new tables through
  OnConditionalCardPageIDNotFound. Single-type opens and tables Page
  Management does not route are exempt.

Wired into al-ui-review (entry gate now covers notification send/recall
outside pages, tokens, high-signal mappings) and registered both pairs in
the ui review-fixtures override.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address review: scope shared notification Ids, routed-table definition, pinned links

- notification-recall-needs-known-id: a fixed Id shared across records is
  valid for one-at-a-time warnings (Sales Line blocked-item, Over-Receipt
  Mgt. pass a fixed Id to SendNotification after Recall); the per-record
  anti-pattern now requires several records' notifications to be visible
  at once. Recall wording follows Learn (including its false-return
  reasons) and cites Base App's recall-before-send practice.
- list-page-document-routing-uses-page-management: definition covers
  opening a routed table directly or after Get; cite archive line lists,
  Copy Document Mgt. ShowSalesDoc/ShowPurchDoc and Office handler; no
  "document types added later" overclaim (GetSalesHeaderPageID has no
  else for the extensible enum); full GetPageID resolution order; drop
  the IRS single-page subscriber.
- al-ui-review: non-page files admitted by the notification clause are
  checked only against notification knowledge; description updated;
  drop the "Document Type" token; case-insensitive token matching; cues
  aligned with both articles.
- Pin BCApps links to 837ef802485ee457e52310d2ecaa08b93d0122fd.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address review: notification recall defect is a lost identity, not an unassigned or generated Id

- notification-recall-needs-known-id: the defect is a Recall() whose Id
  cannot be the sent one (fresh local Notification, new CreateGuid at
  recall time) while neither the sent instance nor its Id is kept. A
  retained global instance (CreateGuid once in OnOpenPage, or Id left for
  Send to assign), a generated Id saved after Send and reassigned before
  Recall, and correctly tracked per-record Ids are explicitly not findings.
  Findings require evidence that the recalled identity differs from or
  cannot recover the sent identity. Notification Lifecycle Mgt. is
  recommended for per-record tracking, not mandatory. Cites VAT Bus. Post.
  Grp. Part, Certificate, and Data Search Lines, and the lifecycle
  helper's Send-then-read-Id sequence.
- good sample: adds a page with a retained global Notification (CreateGuid
  in OnOpenPage, Send in an action, Recall in a later action and
  OnClosePage) and a pageextension that saves the Send-assigned Id and
  recalls it from a later action. Bad sample comments name the lost
  identity.
- al-ui-review: notification cue requires that evidence and lists the
  retained-instance, saved-Id, and direct per-record tracking controls as
  exclusions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

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

@@ -3,7 +3,7 @@ kind: action-skill
 id: al-ui-review
 version: 1
 title: AL UI and accessibility review
-description: Reviews AL page and control add-in UI files against UI text, caption, tooltip, and accessibility guidance from BCQuality.
+description: Reviews AL page and control add-in UI files, and AL code that sends or recalls notifications, against UI text, caption, tooltip, notification, and accessibility guidance from BCQuality.
 inputs: [pr-diff, file-path, folder-path]
 outputs: [findings-report]
 bc-version: [all]
@@ -16,7 +16,7 @@ application-area: [all]
 
 Reviews AL page source and control add-in UI files against the `ui` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is one of the skills composed by `al-code-review`.
 
-UI findings apply to page files — files that declare `PageType = ...`, including `*.Page.al` under the standard file-naming convention — and to JavaScript/CSS/HTML files that implement Business Central control add-ins, including their client-service communication. The skill returns `not-applicable` when the diff contains no page or control add-in changes.
+UI findings apply to page files — files that declare `PageType = ...`, including `*.Page.al` under the standard file-naming convention — to AL code that sends or recalls in-client notifications, and to JavaScript/CSS/HTML files that implement Business Central control add-ins, including their client-service communication. The skill returns `not-applicable` when the diff contains no page, notification, or control add-in changes.
 
 An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. The skill produces a single JSON document conforming to the DO output contract.
 
@@ -39,10 +39,10 @@ Discard files that are not applicable. Retain conditionally applicable files onl
 
 Narrow the relevant files to the subset that applies to the changes under review.
 
-- **UI-file filter.** UI review applies to files declaring `page`, `pageextension`, or `pagecustomization`, and to JavaScript/CSS/HTML that implements a control add-in's rendering or Business Central communication. When the diff contains no such files, return `outcome: "not-applicable"` without evaluating knowledge files.
+- **UI-file filter.** UI review applies to files declaring `page`, `pageextension`, or `pagecustomization`; to any AL object whose changed code calls `Send()` or `Recall()` on a `Notification` variable or calls codeunit `"Notification Lifecycle Mgt."`; and to JavaScript/CSS/HTML that implements a control add-in's rendering or Business Central communication. A non-page AL file admitted only by the notification clause is evaluated only against notification knowledge (`notification-recall-needs-known-id`), not against caption, tooltip, message-text, or client-expression knowledge. When the diff contains no such files, return `outcome: "not-applicable"` without evaluating knowledge files.
 - A new or changed page's name/suffix, primary-key handling, `CardPageID`, `SubPageLink`, `AutoSplitKey`, or `UsageCategory` doesn't match the conventions of its own declared `PageType` — `page-design-must-match-bc-page-type-conventions.md`. A Card page over a composite-key table that supplements a master record, or a supporting/subpage/dialog page intended only to be reached through another workflow and correctly omitting `UsageCategory`, is not this anti-pattern on its own; check whether the page is actually mixing conventions or is meant as a searchable entry point before flagging.
-- For each relevant knowledge file, compute overlap against changed page declarations and control add-in files, weighted toward `Caption`, `ToolTip`, `AboutTitle`, `AboutText`, `OptionCaption`, `ShowCaption`, `InstructionalText`, `GridLayout`, `Style`, `StyleExpr`, promoted action definitions, field importance, page background tasks, DOM creation, ARIA attributes, keyboard/focus handlers, packaged-resource AJAX, and calls from JavaScript into AL.
-- Tokens extracted from the diff (`Caption`, `ToolTip`, `AboutTitle`, `AboutText`, `PageType`, `ShowCaption`, `InstructionalText`, `grid`, `fixed`, `GridLayout`, `Style`, `StyleExpr`, `Importance`, `Promoted`, `Additional`, `area(Promoted)`, `actionref`, `PromotedCategory`, `PromotedOnly`, `PromotedIsBig`, `ShowAs`, `SplitButton`, `fieldgroups`, `DropDown`, `UpdatePropagation`, `EnqueueBackgroundTask`, `OnAfterGetCurrRecord`, `OnAfterGetRecord`, `OnPageBackgroundTaskCompleted`, `OnPageBackgroundTaskError`, `RunPageBackgroundTask`, `Favorable`, `Unfavorable`, `Ambiguous`, `cuegroup`, `controladdin`, `control-add-in`, `usercontrol`, `aria-`, `tabindex`, `keydown`, `focus`, `innerHTML`, `createElement`, `packaged-resource`, `ajax`, `$.get`, `$.ajax`, `XMLHttpRequest`, `xhrFields`, `withCredentials`, `withcredentials`, `InvokeExtensibilityMethod`, `invokeextensibilitymethod`, `skipIfBusy`, `successCallback`, `success-callback`, `errorCallback`, `setInterval`, `JSON.stringify`, `payload`, `throttling`, `reduced-functionality`, `ClientServicesMaxUploadSize`, `&`, `Specifies`, `Message(`, `Confirm(`, `Error(` in a page context, `Disabled`, `Invalid`, `Whitelist`, `Blacklist`, trailing punctuation patterns on captions, `CardPageID`, `AutoSplitKey`, `PageType = Card`, `PageType = List`, `PageType = Worksheet`, `PageType = Document`, `PageType = RoleCenter`, `Enabled`, `Visible`, `Editable`, `in [`, `AccessByPermission`, `ReadPermission`, `WritePermission`).
+- For each relevant knowledge file, compute overlap against changed page declarations and control add-in files, weighted toward `Caption`, `ToolTip`, `AboutTitle`, `AboutText`, `OptionCaption`, `ShowCaption`, `InstructionalText`, `GridLayout`, `Style`, `StyleExpr`, promoted action definitions, field importance, page background tasks, notification send/recall, open-document actions, DOM creation, ARIA attributes, keyboard/focus handlers, packaged-resource AJAX, and calls from JavaScript into AL.
+- Tokens extracted from the diff (`Caption`, `ToolTip`, `AboutTitle`, `AboutText`, `PageType`, `ShowCaption`, `InstructionalText`, `grid`, `fixed`, `GridLayout`, `Style`, `StyleExpr`, `Importance`, `Promoted`, `Additional`, `area(Promoted)`, `actionref`, `PromotedCategory`, `PromotedOnly`, `PromotedIsBig`, `ShowAs`, `SplitButton`, `fieldgroups`, `DropDown`, `UpdatePropagation`, `EnqueueBackgroundTask`, `OnAfterGetCurrRecord`, `OnAfterGetRecord`, `OnPageBackgroundTaskCompleted`, `OnPageBackgroundTaskError`, `RunPageBackgroundTask`, `Favorable`, `Unfavorable`, `Ambiguous`, `cuegroup`, `controladdin`, `control-add-in`, `usercontrol`, `aria-`, `tabindex`, `keydown`, `focus`, `innerHTML`, `createElement`, `packaged-resource`, `ajax`, `$.get`, `$.ajax`, `XMLHttpRequest`, `xhrFields`, `withCredentials`, `withcredentials`, `InvokeExtensibilityMethod`, `invokeextensibilitymethod`, `skipIfBusy`, `successCallback`, `success-callback`, `errorCallback`, `setInterval`, `JSON.stringify`, `payload`, `throttling`, `reduced-functionality`, `ClientServicesMaxUploadSize`, `&`, `Specifies`, `Message(`, `Confirm(`, `Error(` in a page context, `Disabled`, `Invalid`, `Whitelist`, `Blacklist`, trailing punctuation patterns on captions, `CardPageID`, `AutoSplitKey`, `PageType = Card`, `PageType = List`, `PageType = Worksheet`, `PageType = Document`, `PageType = RoleCenter`, `Enabled`, `Visible`, `Editable`, `in [`, `AccessByPermission`, `ReadPermission`, `WritePermission`, `Notification`, `NotificationScope`, `.Send()`, `.Recall()`, `CreateGuid`, `Notification Lifecycle Mgt.`, `SendNotification`, `SendNotificationWithAdditionalContext`, `RecallNotificationsForRecord`, `Page Management`, `PageRun`, `Page.Run(`, `ShowDocument`). Match tokens case-insensitively; Base App writes both `Page.Run(` and `PAGE.Run(`.
 
 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 page element. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
 
@@ -52,6 +52,8 @@ Apply these high-signal mappings before fuzzy topic ranking:
 - An editable page part affects a total, FlowField, or FactBox on the parent but does not set `UpdatePropagation = Both` — `updatepropagation-both-refreshes-main-page`.
 - A page or pageextension `Enabled`, `Visible`, `Editable`, or `StyleExpr` value contains an `in [...]` list (compiler: "InListExpression is not valid for client expressions", AL0573 or AL0322), or replaces one with a procedure call — `page-client-expression-must-not-use-in-list`. Plain `=`/`<>` comparisons joined with `and`/`or`, and `in [...]` inside a trigger or procedure body, are valid; do not flag them.
 - A `RoleCenter` page, or a pageextension whose target is a Role Center, gates a part, action, or field by binding `Visible` or `Enabled` to a procedure that tests a permission, or declares a procedure (AL0569 "A page of type Role Center cannot have procedures", AL0573) — `rolecenter-permission-gating-must-use-accessbypermission`. The target page name is not reliable evidence of its type; confirm it is a Role Center from its `PageType` or the AL0569 diagnostic. Setup- or feature-flag gating inside the part page is not this pattern.
+- Code sends and recalls a `Notification`, and the `Notification` passed to `Recall()` cannot carry the sent `Id`: for example `Send()` on a local variable when a condition holds and `Recall()` on a fresh local variable when it clears, with no `Id` assigned or a new `CreateGuid()` assigned on each call, while neither the sent instance nor its `Id` is kept in a global variable, a record, or `"Notification Lifecycle Mgt."`; or, when notifications for several records must be visible at the same time, they share one fixed `Id` sent and recalled directly — `notification-recall-needs-known-id`. Flag only with evidence that the recalled identity differs from, or cannot recover, the sent identity: trace the `Recall()` variable back to its `Id` assignment and the sent variable forward to where its instance or `Id` is kept. Valid, do not flag: a notification that is never recalled; a global or page-level `Notification` instance sent and later recalled, whether its `Id` came from `CreateGuid()` (for example once in `OnOpenPage`) or from `Send()`; a generated `Id` saved after `Send()` and reassigned before `Recall()`; per-record `Id`s the code tracks correctly itself; one sent through `"Notification Lifecycle Mgt."` (`SendNotification`, `SendNotificationWithAdditionalContext`) without an `Id`; and a fixed `Id` shared across records for a one-at-a-time warning that is recalled before re-send (directly or through `SendNotification`, as in `Sales Line`'s blocked-item notification). Recommend `"Notification Lifecycle Mgt."` for per-record tracking, but do not flag correct direct tracking for not using it.
+- A page action opens a record of `Sales Header`, `Purchase Header`, their archives, or another table that codeunit `"Page Management"` routes (directly or after a `Get`) by switching on its `Document Type` or a similar type field and calling `Page.Run(Page::...)` per branch — `list-page-document-routing-uses-page-management`. Severity `minor`. Opening one known document type directly, a list with a fixed `CardPageID`, and a table that `"Page Management"` does not route (for example `Assembly Header`) are not this pattern.
 
 Once the candidate worklist is known, resolve layer-precedence conflicts per READ and record suppressions.
 
@@ -79,7 +81,7 @@ Outcome selection:
 
 - `completed` — the skill evaluated every worklist item.
 - `no-knowledge` — no applicable UI knowledge survived filtering.
-- `not-applicable` — the diff contains no page, pageextension, pagecustomization, or control add-in implementation files.
+- `not-applicable` — the diff contains no page, pageextension, pagecustomization, notification send/recall, or control add-in implementation files.
 - `partial` — a budget was hit before the worklist was exhausted.
 - `failed` — an unrecoverable error occurred.