Add P0 integration and control add-in runtime guidance (#100)

* Add P0 integration and control add-in guidance

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

Copilot-Session: 02baffe8-0600-430d-81fa-a9993685e7cb

* Correct API part multiplicity guidance

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

Copilot-Session: 02baffe8-0600-430d-81fa-a9993685e7cb

* Refine API part multiplicity guidance

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

Copilot-Session: 1c37924e-9749-4e63-9d58-bd73d659f736

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>

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

@@ -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 render Business Central control add-ins. The skill returns `not-applicable` when the diff contains no page or control add-in UI changes.
+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.
 
 An orchestrator invokes this skill with either a `pr-diff` or a `file-path`. The skill produces a single JSON document conforming to the DO output contract.
 
@@ -39,9 +39,9 @@ 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 control add-in JavaScript/CSS/HTML that changes rendered UI. When the diff contains no such files, return `outcome: "not-applicable"` without evaluating knowledge files.
-- For each relevant knowledge file, compute overlap against changed page declarations and control add-in UI files, weighted toward `Caption`, `ToolTip`, `AboutTitle`, `AboutText`, `OptionCaption`, `ShowCaption`, `InstructionalText`, `GridLayout`, `Style`, `StyleExpr`, action definitions, field-level properties, DOM creation, ARIA attributes, and keyboard/focus handlers.
-- Tokens extracted from the diff (`Caption`, `ToolTip`, `AboutTitle`, `AboutText`, `PageType`, `ShowCaption`, `InstructionalText`, `grid`, `fixed`, `GridLayout`, `Style`, `StyleExpr`, `Favorable`, `Unfavorable`, `Ambiguous`, `cuegroup`, `controladdin`, `usercontrol`, `aria-`, `tabindex`, `keydown`, `focus`, `innerHTML`, `createElement`, `&`, `Specifies`, `Message(`, `Confirm(`, `Error(` in a page context, `Disabled`, `Invalid`, `Whitelist`, `Blacklist`, trailing punctuation patterns on captions).
+- **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.
+- 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`, action definitions, field-level properties, 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`, `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).
 
 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.
 
@@ -53,6 +53,8 @@ When the post-conflict worklist is empty because no applicable UI knowledge exis
 
 For each worklist entry, evaluate the diff against the file's `## Best Practice` and `## Anti Pattern` sections. UI text findings are generally `minor` — they affect localization and polish rather than correctness. Accessibility findings for missing labels, broken grid semantics, semantic color without text meaning, or UI-rendering control add-in changes can be `major`; use `minor` for low-risk manual-review reminders and polish issues.
 
+For packaged-resource requests, flag `$.get` or AJAX/XHR that omits `withCredentials` only when the URL is identifiable as a resource in the control add-in package; do not generalize the rule to external endpoints. For `InvokeExtensibilityMethod`, flag repeated or timer-driven calls that can overlap because they do not wait for the success/error callbacks, and unbounded serialized payloads sent in one call. Prefer bounded chunks serialized through completion callbacks. Do not emit generic browser or JavaScript performance advice.
+
 Set `confidence` to:
 
 - `high` when the detection is based on an unambiguous pattern match (banned term literal, missing "Specifies" opener on a field tooltip, caption exceeding documented limit).
@@ -69,7 +71,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 UI files.
+- `not-applicable` — the diff contains no page, pageextension, pagecustomization, or control add-in implementation files.
 - `partial` — a budget was hit before the worklist was exhausted.
 - `failed` — an unrecoverable error occurred.
 

microsoft/skills/review/al-web-services-review.md

@@ -3,11 +3,11 @@ kind: action-skill
 id: al-web-services-review
 version: 1
 title: AL web services review
-description: Reviews AL source changes against web-services (API page) guidance from BCQuality.
+description: Reviews AL API surfaces and webhook integration handlers against web-services guidance from BCQuality.
 inputs: [pr-diff, file-path]
 outputs: [findings-report]
 bc-version: [all]
-technologies: [al]
+technologies: [al, javascript]
 countries: [w1]
 application-area: [all]
 ---
@@ -27,7 +27,7 @@ Read the BCQuality knowledge index once — the `knowledge-index.json` BCQuality
 Apply the frontmatter matching rules defined in READ (*Frontmatter matching semantics*) against the task context:
 
 - `bc-version` — the target BC version from the PR branch's `app.json` or the orchestrator-supplied version. If unavailable, the dimension is `unknown`.
-- `technologies` — `[al]`.
+- `technologies` — `[al]` or `[javascript]`.
 - `countries` — the countries declared in the consuming app's `app.json`. Default to the orchestrator's configured context; if absent, `unknown`.
 - `application-area` — the union of application areas declared by the changed objects. Pass the actual set; do not substitute `[all]`. If the area cannot be determined from the changes, the dimension is `unknown`.
 
@@ -37,9 +37,10 @@ Discard files that are not applicable. Retain conditionally applicable files (an
 
 Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against:
 
-- The changed AL object names and types — especially page objects declared with `PageType = API`, and any procedure on such a page that exposes a bound action.
-- The changed properties and triggers, weighted toward API page metadata (`APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SourceTable`), CRUD guards (`InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`), the `OnOpenPage` trigger, and `OnValidate` triggers on exposed fields.
-- Tokens extracted from the diff that relate to API surface and behaviour (`PageType`, `API`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SystemId`, `ServiceEnabled`, `WebServiceActionContext`, `SetActionResponse`, `ReadIsolation`, `IsolationLevel`, `ReadCommitted`, `InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`, `SourceTable`).
+- The changed AL object names and types — especially pages declared with `PageType = API`, API page `part` controls, queries declared with `QueryType = API`, and procedures that expose bound actions.
+- The changed properties and triggers, weighted toward API page metadata (`APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SourceTable`, `SourceTableTemporary`), navigation metadata (`SubPageLink`, `Multiplicity`, and visible singleton or collection semantics), CRUD guards (`InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`), the `OnOpenPage` trigger, and `OnValidate` triggers on exposed fields.
+- Webhook subscriber handlers and subscription lifecycle code, especially code that creates or renews subscriptions, handles `validationToken`, schedules from `expirationDateTime`, or targets resources whose eligibility is visible in the diff.
+- Tokens extracted from the diff that relate to API surface and behaviour (`PageType`, `QueryType`, `API`, `api-page`, `page-part`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SystemId`, `SubPageLink`, `subpagelink`, `Multiplicity`, `multiplicity`, `Many`, `ZeroOrOne`, `SourceTableTemporary`, `Job Queue Entry`, `webhook`, `webhookSupportedResources`, `webhook-supported-resources`, `subscriptions`, `notificationUrl`, `validationToken`, `validationtoken`, `expirationDateTime`, `expirationdatetime`, `ServiceEnabled`, `WebServiceActionContext`, `SetActionResponse`, `ReadIsolation`, `IsolationLevel`, `ReadCommitted`, `InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`, `SourceTable`).
 
 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.
 
@@ -55,6 +56,8 @@ For each worklist entry, evaluate the diff against the file's `## Best Practice`
 - When the diff contains code that contradicts a Best Practice without being a full anti-pattern, emit `minor` with the same reference shape.
 - When the skill cannot detect a violation but the file is clearly applicable to the change, emit `info` citing the file. Repository-wide observations MAY omit `location`.
 
+For API parts whose parent declares `ODataKeyFields = SystemId`, detect a child foreign key linked to a parent business field instead of `Field(SystemId)`. Do not apply the SystemId-link rule to APIs intentionally keyed by another field. Omitted `Multiplicity` is valid and means the documented default 1:N collection; never report omission alone. Report an explicit `ZeroOrOne` only when the visible contract clearly intends a collection or deep insert, and report an explicit `Many` only when it clearly intends a singleton. Singleton metadata requires an explicit `ZeroOrOne`; do not infer singleton intent from naming alone. For webhook eligibility, detect `QueryType = API`, `SourceTableTemporary = true`, composite `ODataKeyFields` (including an omitted property when a visible source primary key is composite), Job Queue Entry, and visible system-table sources; do not infer an unknown table number. For lifecycle code, require both create and renew paths to use a handler that returns the query-string `validationToken` verbatim with `200 OK`, and flag renewal scheduling that assumes subscriptions are permanent instead of using `expirationDateTime`. Do not emit generic HTTP or REST advice.
+
 Set `confidence` to:
 
 - `high` when the detection is based on an unambiguous pattern match (identifier, syntax, object type).
@@ -71,7 +74,7 @@ Outcome selection:
 
 - `completed` — the skill evaluated every worklist item; default when the skill finishes normally, including when the resulting `findings` array is empty.
 - `no-knowledge` — no applicable web-services knowledge survived Source, Relevance, configuration filtering, and conflict resolution. `findings` is empty.
-- `not-applicable` — the task context lacks an AL dimension (no AL changes in the diff, or `technologies` filter rejected the task).
+- `not-applicable` — the task context contains no AL API surface, JavaScript webhook subscription lifecycle code, or JavaScript notification handler, or the `technologies` filter rejected the task.
 - `partial` — a time or token budget was hit before the worklist was exhausted. `summary.coverage` reflects the evaluated subset; `outcome-reason` explains the cause.
 - `failed` — an unrecoverable error occurred. `outcome-reason` is required.