diff --git a/custom/README.md b/custom/README.md index d3e8c0a..0efef34 100644 --- a/custom/README.md +++ b/custom/README.md @@ -20,18 +20,13 @@ This layer was seeded by migrating the Business Central AL assets from the `comm | Domain | Articles | Covers | |---|---|---| -| `integration` | 13 | The modern integration pattern catalog: staging through the Integration Message, inbound and outbound idempotency, polling framing records, Business Event versioning and payload safety, correlation propagation, long-running and staged flows, manual resolution, and the hard anti-patterns. | -| `copilot` | 4 | Registering a Copilot capability, calling Azure OpenAI through System.AI, billing type, and authoring a custom agent with the IAgent interfaces. | -| `ux` | 2 | PromptDialog for Copilot Generate UX, and keeping prompt text free of trailing whitespace. | -| `pipelines` | 4 | AL-Go for GitHub CI/CD, settings as the source of truth, environment registration, and headless builds with the AL MCP Server. | -| `security` | 3 | Least-privilege Azure RBAC, Managed Identity over connection strings, and locking environments to an Entra security group. | -| `process` | 3 | Spec-Driven Development: specify before you build, the solution constitution, and mapping features to object ID ranges. | +| `integration` | 15 | The modern integration pattern catalog from the BCTechDays 2026 "Designing Modern Integrations" session: staging through the Integration Message, inbound and outbound idempotency, polling framing records, the single staging endpoint, the wait-loop anti-pattern, Business Event versioning and payload safety, correlation propagation, long-running 202 / status-url flows, staged pipelines, batching trade-offs, error classification, manual resolution, and the hard anti-patterns. | | `api` | 2 | Exposing BC entities as API pages for external agents, and least-privilege MCP tool surfaces. | | `operations` | 2 | SaaS point-in-time restore limits, and inspecting the AL runtime during a debug session. | +| `process` | 1 | Mapping each feature to a reserved AL object ID range during planning. | | `performance` | 1 | Profiling before optimising with the built-in Performance Profiler. | -| `upgrade` | 1 | Gating major version bumps on compatibility testing. | -Many integration, Copilot, and UX articles ship `.good.al` / `.bad.al` companion samples. +Most integration articles ship `.good.al` / `.bad.al` companion samples. ### Skills (`custom/skills/`) @@ -39,7 +34,6 @@ Many integration, Copilot, and UX articles ship `.good.al` / `.bad.al` companion |---|---|---| | `review/` | 14 | Net-new AL reviewers and auditors: multi-tenancy, permission-set, event-subscriber, obsolescence, integration-pattern, upgrade, code-quality, readability, table-refactor, performance, translation, AppSource, and major-release-readiness. Plus `al-extended-review`, a super-skill that composes the six net-new domain reviewers so they dispatch as a group alongside the platform `al-code-review`. | | `testing/` | 10 | The test agent suite (write, validate, run, coverage validate and enforce, user-guide tests, web-client run) plus the release-audit test-guide generator, Page Scripting e2e planning, and Copilot test-driven development. | -| `integration/` | 2 | Validating and reviewing the Azure integration plane (Functions, Service Bus, APIM, Bicep) that BC integrations depend on. | ## How to use diff --git a/custom/knowledge/integration/batch-outbound-work-only-when-the-remote-supports-it.bad.al b/custom/knowledge/integration/batch-outbound-work-only-when-the-remote-supports-it.bad.al new file mode 100644 index 0000000..ed8f63a --- /dev/null +++ b/custom/knowledge/integration/batch-outbound-work-only-when-the-remote-supports-it.bad.al @@ -0,0 +1,47 @@ +// Anti-pattern: "batching" by chaining single calls in one Job Queue run, and +// a real batch with no per-item status. Both strand work on one failure. + +codeunit 50150 "WMS Sender (bad)" +{ + TableNo = "Job Queue Entry"; + + trigger OnRun() + var + IntegrationMessage: Record "Integration Message"; + Client: HttpClient; + Request: HttpRequestMessage; + Response: HttpResponseMessage; + begin + IntegrationMessage.SetRange(Status, IntegrationMessage.Status::New); + if IntegrationMessage.FindSet() then + repeat + // BAD: fifty serial round trips inside one task. This is a wait + // loop with extra steps: one task and its locks are pinned for the + // whole sequence, and a slow remote slows every other queued job. + BuildRequest(IntegrationMessage, Request); + Client.Send(Request, Response); + until IntegrationMessage.Next() = 0; + end; + + // BAD alternative: a genuine batch POST whose response is a single status. + procedure SendBlindBatch(var Request: HttpRequestMessage; Items: List of [Guid]) + var + Client: HttpClient; + Response: HttpResponseMessage; + IntegrationMessage: Record "Integration Message"; + Id: Guid; + begin + Client.Send(Request, Response); + // BAD: one IsSuccessStatusCode for the whole batch. One invalid item + // fails all fifty, and we cannot tell which item to fix or retry. A retry + // re-sends the items that already succeeded. + foreach Id in Items do begin + IntegrationMessage.Get(Id); + if Response.IsSuccessStatusCode() then + IntegrationMessage.Status := IntegrationMessage.Status::Resolved + else + IntegrationMessage.Status := IntegrationMessage.Status::Failed; + IntegrationMessage.Modify(true); + end; + end; +} diff --git a/custom/knowledge/integration/batch-outbound-work-only-when-the-remote-supports-it.good.al b/custom/knowledge/integration/batch-outbound-work-only-when-the-remote-supports-it.good.al new file mode 100644 index 0000000..b0150ea --- /dev/null +++ b/custom/knowledge/integration/batch-outbound-work-only-when-the-remote-supports-it.good.al @@ -0,0 +1,39 @@ +// Best practice: send one bounded batch to a remote that returns per-item +// status, carry a per-item idempotency key inside the batch, and settle each +// Integration Message individually from its own result. A partial failure parks +// only the items that actually failed; a retry of the batch is safe because the +// items that already succeeded carry the same keys. + +codeunit 50150 "WMS Batch Sender" +{ + TableNo = "Job Queue Entry"; + + trigger OnRun() + var + IntegrationMessage: Record "Integration Message"; + BatchBuilder: Codeunit "WMS Batch Builder"; + BatchSize: Integer; + begin + // Size is tuned from telemetry, smaller for stages that lock. Not a constant. + BatchSize := GetTunedBatchSize(); + + IntegrationMessage.SetRange(Direction, IntegrationMessage.Direction::Outbound); + IntegrationMessage.SetRange(Status, IntegrationMessage.Status::New); + IntegrationMessage.SetRange(Type, 'wms-shipment'); + if IntegrationMessage.FindSet() then + repeat + // Each item carries its own Message ID as the idempotency key, + // so re-sending the batch never double-processes a sent item. + BatchBuilder.AddItem(IntegrationMessage."Message ID", IntegrationMessage); + until (IntegrationMessage.Next() = 0) or (BatchBuilder.Count() >= BatchSize); + + // One call. The remote returns a result per item, keyed by Message ID. + SendBatchAndSettleEachItem(BatchBuilder); + end; + + local procedure SendBatchAndSettleEachItem(var BatchBuilder: Codeunit "WMS Batch Builder") + begin + // For each per-item result: set that one message Resolved or Failed. + // A single bad item parks only itself; the rest move on. + end; +} diff --git a/custom/knowledge/integration/batch-outbound-work-only-when-the-remote-supports-it.md b/custom/knowledge/integration/batch-outbound-work-only-when-the-remote-supports-it.md new file mode 100644 index 0000000..6ccccc8 --- /dev/null +++ b/custom/knowledge/integration/batch-outbound-work-only-when-the-remote-supports-it.md @@ -0,0 +1,30 @@ +--- +bc-version: [all] +domain: integration +keywords: [batching, outbound, throughput, partial-failure, per-item-status, telemetry, job-queue] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Batch outbound work only when the remote supports it + +## Description + +Batching several outbound messages into one call reduces per-call overhead, but it couples the fate of the items inside the batch: if the batch of fifty fails, you have to work out which one of the fifty caused it, and most remote APIs return a single success or failure for the whole batch rather than per-item status. Batching is worth it only when the remote exposes a genuine batch endpoint and tells you the outcome of each item, so a single bad item does not strand the other forty-nine. Faking a batch by chaining single calls inside one Job Queue run is worse than not batching at all: it is a synchronous wait loop with extra steps, holding one task and one lock for the whole sequence. + +Batch size is a tuning decision, not a constant. A validate stage can batch large; a posting stage that takes locks should batch small. The right size comes from telemetry on real lock contention and throughput, never from intuition. + +## Best Practice + +Batch in the orchestrator or the sender stage, never inside posting, and only against a remote that accepts a batch and returns per-item status. Keep a per-item idempotency key (the Integration Message id of each item) inside the batch so a retry of the batch does not double-process the items that already succeeded. Mark each message Resolved or Failed individually from the per-item response, so a partial failure parks only the items that actually failed. Start the batch size low and raise it only on the evidence of telemetry, with a smaller size for stages that lock heavily than for stages that only read. See `batch-outbound-work-only-when-the-remote-supports-it.good.al`. + +## Anti Pattern + +Two shapes. First, simulated batching: a Job Queue run that loops `HttpClient.Send` over fifty messages to "batch" them, which is a wait loop that pins one task and serialises fifty round trips. Second, blind batching: a real batch POST whose response is a single status with no per-item detail, so one invalid item fails the whole batch and the code cannot tell which item to fix or retry. The detection signal: a loop of `Client.Send` inside one `OnRun`, or a batch send followed by a single `IsSuccessStatusCode` check that flips every message in the batch to the same status. The consequence is that one bad item strands a whole batch and a retry re-sends the items that already succeeded. See `batch-outbound-work-only-when-the-remote-supports-it.bad.al`. + +## See also + +- `send-an-idempotency-key-on-every-outbound-call.md` +- `accept-async-work-instead-of-synchronous-wait-loops.md` +- `split-multi-step-flows-into-staged-job-queue-entries.md` diff --git a/custom/knowledge/integration/classify-integration-errors-for-resolution.bad.al b/custom/knowledge/integration/classify-integration-errors-for-resolution.bad.al new file mode 100644 index 0000000..a923a22 --- /dev/null +++ b/custom/knowledge/integration/classify-integration-errors-for-resolution.bad.al @@ -0,0 +1,22 @@ +// Anti-pattern: the failure path records only a raw error string and a Failed +// status. There is no error class, so every failure looks the same. On a busy +// Monday an operator must open and read three hundred rows to learn that most +// were timeouts that would have healed on their own, a handful were bad +// addresses, and one was a renamed field that should have paged an engineer. + +codeunit 50140 "Handle Integration Failure" +{ + procedure OnFailure(var IntegrationMessage: Record "Integration Message"; ErrorText: Text) + begin + IntegrationMessage.Status := IntegrationMessage.Status::Failed; + // BAD: raw text, no classification. Nothing tells ops whether to fix + // data, wait for the retry, or escalate. The resolution page shows one + // undifferentiated Failed bucket and time-to-resolve grows with the queue. + IntegrationMessage."Error Message" := CopyStr(ErrorText, 1, MaxStrLen(IntegrationMessage."Error Message")); + IntegrationMessage.Modify(true); + + // BAD: a blanket retry of every Failed row, because the code cannot tell + // transient from permanent. Data errors and contract breaks are retried + // forever, hammering the remote and never reaching a human. + end; +} diff --git a/custom/knowledge/integration/classify-integration-errors-for-resolution.good.al b/custom/knowledge/integration/classify-integration-errors-for-resolution.good.al new file mode 100644 index 0000000..4fa65f3 --- /dev/null +++ b/custom/knowledge/integration/classify-integration-errors-for-resolution.good.al @@ -0,0 +1,39 @@ +// Best practice: on failure, classify the error into one of three actionable +// classes and store it on the Integration Message. Ops then sees a sorted queue +// instead of a wall of raw error text. Rules cover the known codes; an AI +// classifier (via System.AI) buckets the free-text remainder. The class only +// routes the work, it never auto-resolves it. + +enum 50135 "Integration Error Class" +{ + Extensible = true; + + value(0; Unclassified) { Caption = 'Unclassified'; } + value(10; DataError) { Caption = 'Data error'; } // a human fixes the payload + value(20; Transient) { Caption = 'Transient'; } // the scheduled retry heals it + value(30; ContractChange) { Caption = 'Contract change'; } // escalate to the owner +} + +codeunit 50140 "Classify Integration Error" +{ + // Called on the failure path, after Status has been set to Failed. + procedure Classify(var IntegrationMessage: Record "Integration Message") + var + AIClassifier: Codeunit "AI Classifier Wrapper"; + Class: Enum "Integration Error Class"; + begin + // 1) Fast path: deterministic rules over codes we already recognise. + Class := ClassifyByKnownCodes(IntegrationMessage."Error Code"); + + // 2) Fall back to the AI classifier for the free-text messages rules miss. + // The wrapper calls the model through the System.AI module, so the call + // is governed and billed, not a raw HttpClient to a model endpoint. + if Class = Class::Unclassified then + Class := AIClassifier.Classify(IntegrationMessage."Error Message", IntegrationMessage.Type); + + // 3) Store the class so the resolution page can route on it. Advisory only: + // a human still confirms a data fix, the retry job still owns transient. + IntegrationMessage."Error Class" := Class; + IntegrationMessage.Modify(true); + end; +} diff --git a/custom/knowledge/integration/classify-integration-errors-for-resolution.md b/custom/knowledge/integration/classify-integration-errors-for-resolution.md new file mode 100644 index 0000000..7424c5d --- /dev/null +++ b/custom/knowledge/integration/classify-integration-errors-for-resolution.md @@ -0,0 +1,30 @@ +--- +bc-version: [all] +domain: integration +keywords: [error-classification, triage, transient, data-error, contract-change, resolution, ai] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Classify integration errors for resolution + +## Description + +When integration messages fail they pile up, and a single "Failed" status with a raw error string forces an operator to read every one to decide what to do. Most failures fall into one of three classes that demand different responses: a data error (customer not found, invalid currency, wrong VAT code) that a human must fix on the payload, a transient error (timeout, 503, deadlock) that the system should retry with backoff and no human at all, and a contract change (a renamed field, a schema break) that is a code change the integration owner must be paged about. Without the class, transient errors waste human attention while contract breaks sit unescalated, and time-to-resolve grows with the size of the failed queue. + +Classifying each failure and storing the class on the Integration Message turns a Monday pile of three hundred failures into three sorted buckets, each with an obvious next action. The class is one extra field; the value is the routing it enables. + +## Best Practice + +On failure (Status set to Failed with a non-empty error), classify the error into data error, transient, or contract change and write the class to an Error Class field on the Integration Message before ops sees it. The class drives the action: data errors go to the manual resolution page, transient errors are left for the scheduled retry, contract changes raise an alert to the integration owner. A rules table over known error codes handles the common cases; an AI classifier (called through the System.AI module, never a raw model call) buckets the free-text messages that rules miss, which is where most of the time-to-resolve saving comes from. Keep the classifier advisory: the class routes work, it does not auto-resolve it. See `classify-integration-errors-for-resolution.good.al`. + +## Anti Pattern + +A failure handler that sets Status to Failed with only a raw error string and no class, leaving an operator to read and triage every row by hand. The detection signal: an integration error path that writes `Error Message` but has no Error Class (or equivalent category) field and no classification step, so the resolution page shows one undifferentiated Failed bucket. The consequence is that retries that would self-heal get manual attention, genuine data fixes wait behind them, and a contract break that should page an engineer looks identical to a transient timeout. See `classify-integration-errors-for-resolution.bad.al`. + +## See also + +- `make-failed-integration-messages-manually-resolvable.md` +- `version-business-events-and-keep-payloads-stable.md` +- `monitor-external-event-subscription-health.md` diff --git a/custom/knowledge/process/ground-work-in-a-solution-constitution.md b/custom/knowledge/process/ground-work-in-a-solution-constitution.md deleted file mode 100644 index 1237b83..0000000 --- a/custom/knowledge/process/ground-work-in-a-solution-constitution.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -bc-version: [all] -domain: process -keywords: [constitution, brief, tech-design, roadmap, project-context, sdd] -technologies: [al] -countries: [w1] -application-area: [all] ---- - -# Ground work in a solution constitution - -## Description - -A Business Central solution should maintain a small set of durable, high-level documents (the constitution) that every feature spec is grounded in: a project brief (customer and localisation, business processes, goals, non-goals, constraints, success measures), a technical design (architecture, which standard BC modules to reuse, the honest custom-code gaps, the assigned object ID range, the high-level data model, integrations, cross-cutting concerns), and a roadmap (an ordered, numbered feature list with status). These are the documents every agent and engineer reads before doing anything, so individual feature work stays consistent with the agreed direction instead of each feature re-deciding the architecture. - -The constitution exists so that decisions are made once and reused, not re-litigated per feature. The brief fixes the business intent, the technical design fixes the architecture and the object ID range, and the roadmap fixes the order and the numbering that feature folders follow. Because every spec is checked against all three, the documents are where solution-wide consistency actually lives; without them each feature quietly invents its own answer to questions the solution already decided. - -## Best Practice - -Establish the constitution once at the start of a solution, and refresh rather than rewrite it when the business need changes materially, preserving decisions still valid. Interview for missing facts rather than inventing them. Write the brief in plain language with no AL, the technical design favouring reuse of standard BC and justifying every custom-code gap, and the roadmap as a numbered feature list so feature folders match the numbering. Treat the constitution as a human decision: stop for review before proceeding to feature specs. Every feature spec must then be consistent with all three documents. - -## Anti Pattern - -Specifying or building features with no shared brief, technical design, or roadmap to ground them. The consequence is features that contradict each other on architecture, object ID ranges, or which standard modules to reuse, because each one re-decides in isolation. The signal: feature specs that exist with no constitution behind them, or custom AL introduced with no recorded justification for not reusing standard BC. - -## See also - -- `specify-before-you-build.md` -- `map-each-feature-to-an-object-id-range.md` diff --git a/custom/knowledge/process/specify-before-you-build.md b/custom/knowledge/process/specify-before-you-build.md deleted file mode 100644 index 1adb4eb..0000000 --- a/custom/knowledge/process/specify-before-you-build.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -bc-version: [all] -domain: process -keywords: [spec, specification, acceptance-criteria, requirements, sdd, before-implementation] -technologies: [al] -countries: [w1] -application-area: [all] ---- - -# Specify before you build - -## Description - -Before planning or writing any AL for a Business Central feature, write a feature specification that captures the problem, the users and roles, the scope and out-of-scope edges, the user flow, and testable acceptance criteria. The spec is the what and why; it deliberately names no AL objects. Writing it first grounds the work in agreed requirements and makes the result verifiable: each acceptance criterion is concrete enough to become a test, so "done" is something you can check rather than something you argue about after the code exists. - -Keeping AL out of the spec is deliberate, not an omission. Naming objects too early collapses the what into the how and quietly commits the design before anyone has agreed what the feature must do. The acceptance criteria are the load-bearing part: they are written so each one maps to a single test, which means the spec doubles as the test plan and the definition of done is fixed before any code can drift away from it. - -## Best Practice - -For each feature, produce a spec before the plan and before any code. State the problem and the affected users and roles, draw the scope and out-of-scope boundaries, describe the user flow, and write acceptance criteria concrete enough to turn directly into tests. Record genuinely open items under open questions rather than guessing, and stop for human review of the spec before planning or implementing. Keep AL object names out of the spec; those belong to the planning step. At implementation, confirm every acceptance criterion is covered by a passing test. - -## Anti Pattern - -Jumping into AL with only an informal idea of the feature and no written, reviewable acceptance criteria. The consequence is scope that drifts during coding, no shared definition of done, and a result that cannot be verified against agreed requirements. The signal: a feature being implemented with no spec, or a spec that lists vague goals instead of testable acceptance criteria, or one that has already committed to AL object names before the what and why are agreed. - -## See also - -- `ground-work-in-a-solution-constitution.md` -- `map-each-feature-to-an-object-id-range.md` diff --git a/custom/skills/integration/azure-integration-review.md b/custom/skills/integration/azure-integration-review.md deleted file mode 100644 index 5138e60..0000000 --- a/custom/skills/integration/azure-integration-review.md +++ /dev/null @@ -1,73 +0,0 @@ ---- -kind: action-skill -id: azure-integration-review -version: 1 -title: Azure integration review -description: The integration-plane review playbook for the Azure side of a BC integration, pairing with the BC-side validator. -inputs: [repository, pr-diff, file-path] -outputs: [findings-report] -bc-version: [all] -technologies: [bicep, csharp] -countries: [w1] -application-area: [all] ---- - -# Azure integration review - -Reviews the Azure side of a Business Central integration as a playbook: the webhook receiver that catches a storefront event, the Logic App that routes a shipment to a WMS, the Service Bus topic that carries Business Events, the Durable Function that schedules a retry, and the Bicep, ARM, or Terraform that provisions them. The integration plane earns its keep by keeping retry, dead-letter, observability, and credential handling outside BC, so BC stays free of external credentials and third-party schema changes do not break it. This skill pairs with the BC-side `azure-integration-validator` so the inbound, outbound, long-running, and manual arrows line up end to end. This is a leaf action skill: it invokes no sub-skills. - -An orchestrator invokes this skill with a `repository`, a `pr-diff` (a change to the integration plane), or a `file-path` (a specific artifact to review). It produces a single JSON document conforming to the DO output contract. - -## Source - -Read the BCQuality knowledge index once (the `knowledge-index.json` Entry's preparation step regenerates over the live, already-filtered clone). Take the index entries whose `domain` is `integration` or `security` as the citable candidate set across every enabled layer; the playbook's rules about receivers, idempotency, retry, dead-letter, correlation, observability, long-running poll, subscription health, and secret handling can match a curated file. Do not open individual article files at this step; open an article's full body only once it enters the Worklist below. The Azure-side house rules are largely not covered by a BC-focused curated file, so most findings are agent findings within this skill's domain (see Action). - -## Relevance - -Apply the frontmatter matching rules defined in READ against the task context: - -- `bc-version`: the BC version the plane integrates with, or `unknown` if unavailable. -- `technologies`: `[bicep, csharp]` (the infrastructure-as-code and Function handler code the playbook reviews). -- `countries`: the consuming solution's declared countries, or `unknown`. -- `application-area`: the application areas of the integration, or `unknown`. - -Discard files that are not applicable. Retain conditionally applicable files (any dimension `unknown`) only when configuration permits; findings derived from them have `confidence` no higher than `medium`, and the finding `message` names the unknown dimensions. - -## Worklist - -Narrow to the artifacts present and the playbook rules each draws. If the repository has no Azure artifacts, report that plainly rather than inventing findings. Read Bicep, ARM, and Terraform that provision Functions, Logic Apps, APIM, Service Bus, and Storage; Logic App and workflow definitions; Function app config (`host.json`, `function.json`); APIM policy XML; and pipeline files. Place each artifact on the four arrows (inbound, outbound, long-running, manual) and build the worklist against the playbook rules: receiver stages to BC, receiver acknowledges fast, idempotency key forwarded, idempotent consumer, retry in the plane, dead-letter configured, transient versus permanent classification, durable retry not a tight loop, correlation header on every hop, observability wired, 202 status poll for long-running, subscription health check, secrets in Key Vault, Managed Identity, HTTPS only. Also worklist the cross-checks with the BC side: an outbound BC call's idempotency key the plane must forward, a BC Correlation ID the plane must carry, a parked long-running message the plane must drive the poll for, and Business Event subscriptions the plane must monitor. - -A curated `integration` or `security` file enters the worklist when its `keywords` intersect these tokens. Read its full body only after it makes the worklist. Resolve layer-precedence conflicts per READ and record dropped files in `suppressed`. - -## Action - -For each worklisted artifact and rule, evaluate the plane against the playbook. Where a curated `integration` or `security` knowledge file states the rule, emit a knowledge-backed finding citing it: `id` equal to the file path, `severity` up to `blocker` only when the file states a platform-level guarantee, otherwise `major`, `confidence` `high` for an unambiguous match. Where no curated file covers the rule (the common case for the Azure-side checks), emit an agent finding within this skill's domain: `references: []`, `id` slug prefixed `agent:` (for example `agent:az-receiver-stages-to-bc`, `agent:az-correlation-header`, `agent:az-subscription-health-check`), `confidence` capped at `medium`, `severity` capped at `minor`, and a self-contained `message` naming the artifact, what the playbook expects, what the artifact does, and the concrete fix. When the underlying impact would otherwise be a blocker (a receiver running BC business logic inline, a missing dead-letter path swallowing poison messages, a stripped correlation id breaking end-to-end tracing), keep the emitted `severity` at `minor` but say so plainly in the `message` and flag that the rule should be promoted to a curated knowledge file before it can gate. Set `suggested-code` when the fix is a mechanical edit to a contiguous artifact span; otherwise set `suggested-code-omission-reason`. Hold every agent candidate to the precision bar in `skills/do.md`: steelman that the plane's choice is deliberate before emitting, and omit when in doubt. - -Outcome selection: `completed` when every worklisted artifact was reviewed (including a clean plane with empty `findings`); `not-applicable` when the repository contains no Azure integration artifacts (report this in `outcome-reason`); `no-knowledge` when artifacts exist but no curated knowledge survived and no agent finding was raised; `partial` or `failed` per the DO contract with `outcome-reason`. - -## Output - -Output conforms to the DO output contract. Playbook rules with no curated backing are agent findings (`references: []`, `agent:` id mirroring the `az-*` rule, severity capped at `minor`, gating impact stated in the message); findings citing an `integration` or `security` file carry that file path as `id` and primary reference. - -```json -{ - "skill": { "id": "azure-integration-review", "version": 1 }, - "outcome": "completed", - "summary": { - "counts": { "blocker": 0, "major": 0, "minor": 1, "info": 0 }, - "coverage": { "worklist-size": 3, "items-evaluated": 3 } - }, - "findings": [ - { - "id": "agent:az-correlation-header", - "severity": "minor", - "message": "functions/ShipmentRouter reads the inbound message but does not set the Correlation ID on the outbound Service Bus header, so a trace cannot be joined across BC, the plane, and the WMS. Read the correlation id from the inbound message and set it on the Service Bus message header and every outbound HTTP header, and log it at each step. Promote to a curated rule before it can gate.", - "location": { "file": "functions/ShipmentRouter/run.csx" }, - "references": [], - "confidence": "medium", - "suggested-code-omission-reason": "fix spans message construction and logging, not a single contiguous span" - } - ], - "suppressed": [] -} -``` diff --git a/custom/skills/integration/azure-integration-validator.md b/custom/skills/integration/azure-integration-validator.md deleted file mode 100644 index 4851d5f..0000000 --- a/custom/skills/integration/azure-integration-validator.md +++ /dev/null @@ -1,97 +0,0 @@ ---- -kind: action-skill -id: azure-integration-validator -version: 1 -title: Azure integration validator -description: Validates the Azure plane for a BC integration, checking receivers, Service Bus, Durable Functions, retry/dead-letter, idempotency, correlation, secrets, and subscription health. -inputs: [repository, file-path, pr-diff] -outputs: [findings-report] -bc-version: [all] -technologies: [bicep, csharp] -countries: [w1] -application-area: [all] ---- - -# Azure integration validator - -Validates the Azure component build that sits between Business Central and external systems: webhook receivers (Functions, Logic Apps, APIM), Service Bus topics and queues, Durable Functions, and the Bicep, ARM, or Terraform that provisions them. The single question is whether, when BC stages a message or fires an event, the plane delivers it reliably, traceably, and exactly once. It reads the artifacts and reports where the plane fails its half of the contract; the developer chooses which fixes to apply. This is a leaf action skill: it invokes no sub-skills. - -An orchestrator invokes this skill with a `repository`, a `file-path` (a narrow scope such as the storefront webhook Function), or a `pr-diff` (a change touching Azure integration artifacts). It produces a single JSON document conforming to the DO output contract. - -## Source - -Read the BCQuality knowledge index once (the `knowledge-index.json` Entry's preparation step regenerates over the live, already-filtered clone). Take the index entries whose `domain` is `integration` or `security` as the citable candidate set across every enabled layer: receiver-staging, idempotency, retry-and-dead-letter, correlation, observability, secret-handling, and managed-identity rules can back a finding. Do not open individual article files at this step; open an article's full body only once it enters the Worklist below. The integration-plane house rules (the `az-*` checks below) are largely Azure-side and rarely map onto a BC-focused curated file, so most findings here are agent findings within this skill's domain (see Action). - -## Relevance - -Apply the frontmatter matching rules defined in READ against the task context: - -- `bc-version`: the target BC version the plane integrates with, or `unknown` if unavailable. -- `technologies`: `[bicep, csharp]` (the infrastructure-as-code and Function handler code the checks actually touch). -- `countries`: the consuming solution's declared countries, or `unknown`. -- `application-area`: the application areas of the integration, or `unknown`. - -Discard files that are not applicable. Retain conditionally applicable files (any dimension `unknown`) only when configuration permits; findings derived from them have `confidence` no higher than `medium`, and the finding `message` names the unknown dimensions. - -## Worklist - -Narrow to the Azure artifacts present and the integration-plane checks each draws. If the repository contains no Azure integration artifacts, do not invent findings: report that plainly. Read Bicep (`*.bicep`), ARM (`azuredeploy.json`, `*.template.json`), Terraform (`*.tf`), Logic App and workflow definitions (`workflow.json`, `*.logicapp.json`), Function app config (`host.json`, `function.json`, retry and binding config) and handler source, and APIM policy XML (inbound, backend, outbound, on-error). Build the worklist against these checks: - -- Receiver stages to BC and does not run BC business logic or block on BC completion inline. -- Receiver acknowledges fast (a 2xx, or 202 for async); no long synchronous work inside it. -- Idempotency key forwarded: outbound calls carry the BC Message ID as `Idempotency-Key`; inbound receivers forward the source system id; the plane does not strip it. -- Idempotent consumer: receivers and queue consumers dedup on the event id or business key before a second side effect. -- Retry in the plane: explicit on the Logic App action, the Function `host.json`, or the Service Bus delivery count, not a silent default or a hand-written loop. -- Dead-letter configured: Service Bus queues and subscriptions enable dead-lettering with a defined max delivery count. -- Transient versus permanent: retries 408, 429, 5xx, and timeouts; routes 4xx and invalid data to DLQ or alert. Retrying a 4xx forever is the most severe failure. -- Durable retry, not a tight loop: long retries scheduled by a Durable Function or a Logic App timer carrying the same idempotency key. -- Correlation header: the Correlation ID is read from the inbound message, set on the Service Bus header and every outbound HTTP header, and logged at each step. -- Observability wired: Functions and Logic Apps have Application Insights or equivalent. -- 202 status poll: a long-running external process is parked and polled or callback-driven, then written back to the same Integration Message; no synchronous connection held open for hours. -- Subscription health check: where the plane relies on BC Business Event subscriptions, a scheduled job lists them and alerts on drift, since they expire silently. -- Secrets in Key Vault: credentials and connection strings come from Key Vault via Managed Identity, not inline. A literal secret is the most severe failure. -- Managed Identity: plane-to-BC and plane-to-resource auth uses Managed Identity where supported. -- HTTPS only: receivers and Function apps enforce HTTPS with a current TLS minimum; the Function is not public where APIM is the intended front door. - -A curated `integration` or `security` file enters the worklist when its `keywords` intersect these tokens. Read its full body only after it makes the worklist. Resolve layer-precedence conflicts per READ and record dropped files in `suppressed`. - -## Action - -For each worklisted artifact, evaluate it against the checks. Where a curated `integration` or `security` knowledge file states the rule (for example a secret-handling or idempotency rule), emit a knowledge-backed finding citing it: `id` equal to the file path, `severity` up to `blocker` only when the file states a platform-level guarantee, otherwise `major`, `confidence` `high` for an unambiguous match. Where no curated file covers the integration-plane check (the common case), emit an agent finding within this skill's domain: `references: []`, `id` slug prefixed `agent:` mirroring the house rule (for example `agent:az-secrets-in-keyvault`, `agent:az-dead-letter-configured`, `agent:az-classify-transient-vs-permanent`), `confidence` capped at `medium`, `severity` capped at `minor`, and a self-contained `message` naming the artifact and line, what is wrong, and the concrete fix. When the underlying impact would otherwise be a blocker (a literal secret checked into source, a 4xx retried forever, a receiver running BC logic inline, a stripped idempotency key), keep the emitted `severity` at `minor` but say so plainly in the `message` and flag that the check should be promoted to a curated rule before it can gate. Set `suggested-code` when the fix is a mechanical edit to a contiguous artifact span (a Key Vault reference replacing a literal, a `maxDeliveryCount` plus dead-letter setting on a subscription); otherwise set `suggested-code-omission-reason`. Hold every agent candidate to the precision bar in `skills/do.md`: steelman that the configuration is deliberate (the secret may be a non-sensitive placeholder, the retry default may be intended) before emitting, and omit when in doubt. - -Outcome selection: `completed` when every worklisted artifact was evaluated (including a clean plane with empty `findings`); `not-applicable` when the repository contains no Azure integration artifacts (report this in `outcome-reason`); `no-knowledge` when artifacts exist but no curated knowledge survived and no agent finding was raised; `partial` or `failed` per the DO contract with `outcome-reason`. - -## Output - -Output conforms to the DO output contract. Integration-plane checks with no curated backing are agent findings (`references: []`, `agent:` id mirroring the `az-*` rule, severity capped at `minor`, gating impact stated in the message); findings citing an `integration` or `security` file carry that file path as `id` and primary reference. - -```json -{ - "skill": { "id": "azure-integration-validator", "version": 1 }, - "outcome": "completed", - "summary": { - "counts": { "blocker": 0, "major": 0, "minor": 2, "info": 0 }, - "coverage": { "worklist-size": 4, "items-evaluated": 4 } - }, - "findings": [ - { - "id": "agent:az-secrets-in-keyvault", - "severity": "minor", - "message": "infra/main.bicep line 142: the WMS API key is a literal string in the Function app settings, checked into source and visible in deployment history. Impact is a blocker: move the key to Key Vault and reference it via @Microsoft.KeyVault(...), granting the Function access through its Managed Identity. Promote to a curated rule before it can gate.", - "location": { "file": "infra/main.bicep", "line": 142 }, - "references": [], - "confidence": "medium", - "suggested-code-omission-reason": "fix requires creating a Key Vault secret and a reference whose name is not derivable from the diff" - }, - { - "id": "agent:az-dead-letter-configured", - "severity": "minor", - "message": "infra/servicebus.bicep line 60: the shipments subscription sets neither deadLetteringOnMessageExpiration nor maxDeliveryCount, so poison messages loop or vanish. Enable dead-lettering with a defined max delivery count and add a consumer or alert on the DLQ.", - "location": { "file": "infra/servicebus.bicep", "line": 60 }, - "references": [], - "confidence": "medium" - } - ], - "suppressed": [] -} -```