Commit graph

75 commits

Author SHA1 Message Date
dcenic
8ff2326b61
Merge pull request #187 from microsoft/dcenic-validate-unauthenticated-responses
Some checks are pending
Validate knowledge index / validate-index (push) Waiting to run
Validate AL review fixtures / validate-review-fixtures (push) Waiting to run
Validate skill index and report schemas / validate-contract (push) Waiting to run
Validate frontmatter and structure / validate (push) Waiting to run
Code reviewer skill for recognizing and validating unauthenticated responses
2026-09-16 09:50:14 +02:00
Djordje Cenic
5bda054927 Link samples using the READ markdown-link convention
Use [\slug.good.al\](slug.good.al) form so tools/Knowledge-Retrieval.ps1
Assert-SampleLink validation passes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-09-15 13:48:59 +02:00
Djordje Cenic
58b3be23ab Name the three required checks explicitly: response size, schema compliance, content integrity
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-09-15 13:19:37 +02:00
Djordje Cenic
2c45021cb3 Add security knowledge: validate unauthenticated endpoint responses
New remedial article for spotting when AL calls an endpoint that does not
authenticate itself to the client (bare HttpClient.Get, blank SOAP SecretText,
post-DisableHttpsCheck HTTP) and requires the response to be size-, schema-, and
request/response-integrity-validated before it is trusted. Includes the
BC-specific false-positive clarifications (platform buffers the full body, so an
in-AL size check after buffering is correct; no DNS-rebinding/bounded-read demand;
HTTPS not always enforceable) plus good/bad AL samples.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-09-15 13:16:53 +02:00
Stefano Demiliani
861f53dd97
Add query filter semantics guidance (#186)
Some checks are pending
Validate knowledge index / validate-index (push) Waiting to run
Validate AL review fixtures / validate-review-fixtures (push) Waiting to run
Validate skill index and report schemas / validate-contract (push) Waiting to run
Validate frontmatter and structure / validate (push) Waiting to run
2026-09-15 12:51:15 +02:00
waldo
b7617fb48a
knowledge(events): ChangeCompany leaves triggers and trigger-event subscribers running in the calling company (#152)
* knowledge(events): ChangeCompany leaves triggers and trigger-event subscribers running in the calling company

ChangeCompany redirects only the data access of a record variable; Learn states that triggers still run in the current company. The database trigger events are raised on every database operation and only pass RunTrigger to the subscriber, so Insert(false) after ChangeCompany still runs every subscriber in the calling company. Generated code either assumes the record 'becomes' a target-company record, or switches RunTrigger off and hand-copies the trigger logic, leaving the subscribers writing to the wrong company; none of the tested runs reached StartSession with the company parameter.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* knowledge(events): qualify StartSession async semantics and fix concurrency-unsafe sample key

Addresses PR #152 review: StartSession is a fire-and-forget background
session (Ok reports only whether it started, not whether the codeunit
succeeded, and errors inside it do not propagate), so the Best Practice
now scopes the recommendation and calls out the durable status/error
channel a synchronous-success write needs. The good sample's
FindLast()+1 entry-number pattern raced under concurrent background
sessions; switched to AutoIncrement, which the platform guarantees is
unique across concurrent transactions.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: waldo1001 <12088142+waldo1001@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-15 12:34:28 +02:00
Stefano Demiliani
b545b22fb9
Add reporting review guidance and evaluation fixtures (#183)
* knowledge(performance): add job queue reliability guidance

* Address Job Queue review feedback

* Address Job Queue routing review feedback

* Encode job queue conflict in fixtures

* Add reporting review guidance and fixtures

* Fix reporting article reference
2026-09-15 10:38:32 +02:00
dayland
852a676285
Fix Job Queue sample links (#184)
Use the required READ-convention Markdown links so knowledge retrieval can associate all new samples with their articles.

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

Co-authored-by: dayland <dayland@microsoft.com>
Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
2026-09-15 09:32:40 +02:00
Stefano Demiliani
8c26ba4e76
Add Job Queue reliability and scheduling guidance (#148)
Some checks are pending
Validate knowledge index / validate-index (push) Waiting to run
Validate AL review fixtures / validate-review-fixtures (push) Waiting to run
Validate frontmatter and structure / validate (push) Waiting to run
* knowledge(performance): add job queue reliability guidance

* Address Job Queue review feedback

* Address Job Queue routing review feedback

* Encode job queue conflict in fixtures
2026-09-14 12:44:30 +02:00
Jesper Schulz-Wedde
c12b2f0a88
Separate analyzer rules from BCQuality knowledge (#178)
Some checks are pending
Validate knowledge index / validate-index (push) Waiting to run
Validate AL review fixtures / validate-review-fixtures (push) Waiting to run
Validate frontmatter and structure / validate (push) Waiting to run
Retire deterministic compiler and analyzer duplicates, remove their review routing, and clarify the admission test for contextual analyzer knowledge.

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-09-11 09:26:14 +02:00
Jesper Schulz-Wedde
ac9e4fd9a2
Complete partner contribution and knowledge consumption guides (#176)
* Improve partner onboarding and documentation navigation

Lead with a complete plugin quick start and add task-oriented usage, troubleshooting, customization, and contribution guides. Preserve the broader plugin framing, correct conflicting contract guidance, support Agents folder reviews, and align repository validation. Convert existing sample references to clickable links without changing knowledge rules.

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

* Complete partner contribution and knowledge consumption guides

Explain direct reading, supplied skills, and custom-agent consumption. Add a first-contribution walkthrough and concrete integration bootstrap, and clarify SetLoadFields guidance with authoritative sources and explicit review heuristics.

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

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-09-11 09:09:53 +02:00
Jesper Schulz-Wedde
2b5550c346
Improve partner onboarding and documentation navigation (#174)
Some checks failed
Validate knowledge index / validate-index (push) Has been cancelled
Validate AL review fixtures / validate-review-fixtures (push) Has been cancelled
Validate frontmatter and structure / validate (push) Has been cancelled
Lead with a complete plugin quick start and add task-oriented usage, troubleshooting, customization, and contribution guides. Preserve the broader plugin framing, correct conflicting contract guidance, support Agents folder reviews, and align repository validation. Convert existing sample references to clickable links without changing knowledge rules.

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-09-09 17:31:03 +02:00
Jesper Schulz-Wedde
8584217c75
Clarify page field caption and tooltip inheritance (#160)
Some checks failed
Validate knowledge index / validate-index (push) Has been cancelled
Validate AL review fixtures / validate-review-fixtures (push) Has been cancelled
Validate frontmatter and structure / validate (push) Has been cancelled
* Clarify page field caption and tooltip inheritance

Prevent redundant page-level properties by documenting inherited captions and BC24/runtime 13.0 table-field tooltips. Correct companion examples and version-scoped tooltip guidance.

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

* Address tooltip quality and knowledge scope review feedback

Require useful, behavior-grounded tooltip text rather than caption repetition, improve the samples, and explain why compiler feedback does not prevent redundant page captions.

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

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-09-07 15:13:35 +02:00
Wenjie Fan
1a5afdc0eb
Merge pull request #132 from microsoft/gggdttt-refine-self-improvement-guidance
Some checks failed
Validate knowledge index / validate-index (push) Has been cancelled
Validate AL review fixtures / validate-review-fixtures (push) Has been cancelled
Validate frontmatter and structure / validate (push) Has been cancelled
Refine self-improvement review guidance
2026-09-03 15:42:39 +02:00
Jesper Schulz-Wedde
4f0a13a801
Promote knowledge for Microsoft review skills (#153)
Move canonical knowledge for Microsoft-owned review domains into the Microsoft layer and document the skill/knowledge co-location policy.

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

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Copilot-Session: 2a6ea875-d38e-4f30-aadb-0d606f9be231
2026-09-03 15:06:01 +02:00
Kilian Seizinger
82422f94c9
Avoid Public Event publisher (#144)
* Avoid Public Event publisher

* knowledge(events): scope public-publisher detection to same-app raisers

The detection rule flagged every public event publisher, including ones
deliberately public so a sibling app can raise them - a contract `internal`
cannot express across app boundaries. Scope the finding to publishers that
are public although only their own app raises them, and record the
cross-app case as a valid Best Practice option.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0b67b90d-e4b4-4b92-9684-726c72c43b3f

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0b67b90d-e4b4-4b92-9684-726c72c43b3f
2026-09-02 16:07:18 +02:00
Jesper Schulz-Wedde
3e848d1ec2
knowledge(performance): align SetLoadFields placement with AL Guidelines (#130)
* knowledge(performance): align SetLoadFields placement with AL Guidelines (#120)

The AL Guidelines mark `SetLoadFields` placed before `SetRange`/`SetFilter`
as bad code and recommend filters first, while the BCQuality samples used
the opposite order — contradictory guidance across two Microsoft repos.

Per Learn (`Record.SetLoadFields`), "fields that are filtered upon are
always loaded", so the two orders produce an identical projection. The
upstream rule is a readability convention: keep `SetLoadFields` adjacent
to the read it governs.

- Reorder filters ahead of `SetLoadFields` in the six affected AL samples.
- State the placement convention in the Best Practice section.
- Record in Description that order does not change the projection, and
  that only a fieldless `SetLoadFields()` or a later overwriting call does.
- Add an Anti Pattern note so review agents treat the reverse order as a
  readability observation, never a performance defect.

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

* Clarify partial-record projection changes

Document AddLoadFields, SetBaseLoadFields, and Reset alongside SetLoadFields so the statement-order guidance does not imply those APIs leave the projection unchanged.

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

Copilot-Session: 89dba8c8-6529-4b60-956f-875a59be499d

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 89dba8c8-6529-4b60-956f-875a59be499d
2026-09-02 14:44:09 +02:00
wenjiefan
c213f1495e Exempt optional notification handlers from the HandlerFunctions execution rule
The narrowed UI-handler guidance still stated the execution rule without the
qualifier the linked Microsoft reference uses. The article said every listed
handler must execute at least once, and the testing leaf skill asked for
`[HandlerFunctions(...)]` to match the invoked handlers exactly. The reference
says every *nonoptional* listed handler must execute, and that send-notification
and recall-notification handlers can be optional. As written, an agent could
flag a deliberately unused optional notification handler.

The discriminator is narrower than the handler type. Both
`[SendNotificationHandler([HandlerIsOptional: Boolean])]` and
`[RecallNotificationHandler([HandlerIsOptional: Boolean])]` take an explicit
optionality argument, so `[SendNotificationHandler(true)]` is exempt while the
same attribute written without the argument stays nonoptional like every other
handler type. Keying the exemption on the argument rather than the type keeps it
checkable from the diff and avoids the opposite false positive, where an agent
stops flagging genuinely nonoptional notification handlers.

Changes:

- The article now states the nonoptional qualifier, explains that optionality is
  declared rather than inferred, and adds an explicit do-not-flag clause. That
  clause also forbids proposing removal, because the listed entry is what keeps
  the test passing on the runs where the notification does fire.
- The testing leaf skill carries the same boundary in its `ui-handlers-in-tests`
  cue, and its mechanical-fix list no longer allows removing a listed optional
  notification handler as a one-click suggestion.
- `SendNotificationHandler` and `RecallNotificationHandler` were missing from the
  skill's testing token list, so notification handlers were not reliably
  surfaced to the relevance step at all. Both are now listed.
- The good sample gains a test that lists an unreached
  `[SendNotificationHandler(true)]`; the bad sample gains the mirror image, an
  unreached `[SendNotificationHandler]` with no optionality argument. The pair
  differs only by that argument, which is the point.
- `evaluation/review-fixtures.json` pins the testing domain to
  `ui-handlers-in-tests` so the boundary is exercised: the good sample is the
  clean control at `minimumCleanRate` 1.0 and the bad sample is the expected
  finding. Keywords were retagged with `notification` and `optional-handler`.

validate_frontmatter.py reports 0 errors; Test-ReviewFixtures.ps1 passes with
32 cases across 16 leaf domains and resolves the testing fixture to this article.
2026-09-02 11:46:13 +02:00
Wenjie Fan
35a7e72f12
knowledge: three false-positive guards from BCApps PR 10277, 10278 and 10346 (#146)
data-modeling: add insert-only-transfer-may-rely-on-caller-cleanup. A filter-and-insert transfer routine was reported for stale rows and duplicate keys even though the field OnValidate trigger calls a sibling cleanup procedure that clears the same range immediately before it. Deciding this requires reading the caller, so the article asks reviewers to trace call sites and keeps uncleared or mismatched-filter paths reportable.

appsource: scope two-level-namespace-replaces-object-affix-not-extension-member-affix to apps that actually configure a mandatory affix. AS0011 only runs when AppSourceCop is enabled with a mandatory affix; a first-party in-box app that ships no such configuration is not subject to it. The member-affix requirement itself is unchanged for apps that do configure one.

testing: allow permission-tests-must-lower-the-execution-context to accept a composed role. The article demanded the exact permission set under test be assigned directly, so a test that lowered permissions through a role including that set and then asserted WritePermission was false was reported as a coverage gap. What matters is the effective context plus a boundary assertion, not which object the test names.

Co-authored-by: wenjiefan <wenjiefan@microsoft.com>
2026-09-02 11:25:19 +02:00
Wael
f027e28f83
knowledge: improve review precision from BCApps PR 10080 feedback (#128)
* knowledge: improve review precision from BCApps PR 10080 feedback

* Update microsoft/knowledge/appsource/object-affixes-prevent-collisions.md

Co-authored-by: Natalie Karolak, MVP <34504100+NKarolak@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Jesper Schulz-Wedde <JesperSchulz@users.noreply.github.com>
Co-authored-by: Natalie Karolak, MVP <34504100+NKarolak@users.noreply.github.com>
2026-09-02 11:16:30 +02:00
wenjiefan
5016962b40 Align the IsHandled article slug, keywords and good sample with its narrowed scope
The article was rewritten to say a reset is required only when the value can
carry over, and its H1 was updated to match, but three artefacts still carried
the old "always initialize to false" premise:

- The slug still read `initialize-ishandled-to-false-before-publishing`, which
  contradicts the body. The slug is not cosmetic: Build-KnowledgeIndex.ps1
  ranks candidates on keywords, frontmatter dimensions, domain, path and title,
  so a stale path pushes selection back toward the behaviour this change
  narrows. Renamed to `reset-ishandled-only-when-the-value-can-carry-over`,
  following the existing precedent for conditional slugs such as
  `unreleased-symbol-change-is-not-a-breaking-change`.
- Keywords still listed `initialization` and `deterministic` and omitted
  `false-positive`, the tag this repository uses for suppression articles.
  Replaced with `carry-over` and `loop-iteration` and added `false-positive`.
- The good sample demonstrated only the "prefer separate fresh locals" clause
  and contained no reset at all, so the article's headline case had no positive
  example. It was also asymmetric with the bad sample, which gained a loop
  procedure showing a local that carries `true` into the next iteration.

Added the matching loop procedure to the good sample: a local declared outside
the loop is reset at the top of each iteration. That case cannot be solved by
introducing another local, because AL has no block scope, so it is the only
shape that demonstrates the reset the article still requires. It also gives the
engine the correct `suggested-code` shape for the loop finding; without it the
one-click fix adapted from the good sample would propose splitting the variable
rather than adding one line.

Also renamed the sample codeunits from "IsHandled Init ..." to
"IsHandled Carry Over ...", and updated the two references to the old slug:
the events leaf skill cue and the events pin in evaluation/review-fixtures.json.

validate_frontmatter.py reports 0 errors; Test-ReviewFixtures.ps1 passes with
32 cases across 16 leaf domains and resolves the events fixture to the renamed
article.
2026-09-02 11:08:16 +02:00
Wenjie Fan
00c9307483
knowledge(breaking-changes): adding a parameter to an event publisher is not a signature break (#139)
* knowledge(breaking-changes): exclude appended event parameters from the signature-change rule

The article's detection guidance flags 'a parameter added' on any shipped procedure. Applied to an event publisher that is bound to rather than called, this produced a false positive on BCApps PR 10278, where a trailing var parameter was appended to the existing IntegrationEvent OnAfterOpenForRecRef and the developer twice replied that adding a parameter to an existing integration event is not a breaking change.

It also contradicted events/add-new-event-parameters-at-the-end, which already states that existing subscribers still bind to the leading parameters. Scope the rule to called procedures, carve out appended event parameters as additive, and keep every other event signature edit - removal, reorder, retype, var flip - in scope. Point the IsHandled case at events/do-not-add-ishandled-to-an-existing-event, which owns that semantic concern.

* fix: event parameter additions are additive at any position, not only when appended

The first revision justified the carve-out with leading-prefix binding and limited it to parameters appended at the end. Verified against shipping BCApps code that AL binds subscriber parameters by name, not position, so an added parameter is additive wherever it is placed. Also corrects the cross-reference to events/adding-a-parameter-to-an-event-is-not-a-breaking-change, which already states this rule, and drops reordering from the list of edits that break binding.

* Route event signature edits to the analyzer-backed events article

Address review feedback: name AS0025, AS0063 and AS0077 for the var and rename cases instead of claiming them in the breaking-changes article, and point at events/treat-local-and-internal-events-as-subscriber-contracts which already owns them.

---------

Co-authored-by: wenjiefan <wenjiefan@microsoft.com>
2026-09-02 09:24:54 +02:00
Wenjie Fan
85ceaaa1a4
knowledge(style): event subscribers bind by parameter name, so a shorter list is not a mismatch (#138)
Some checks failed
Validate knowledge index / validate-index (push) Has been cancelled
Validate AL review fixtures / validate-review-fixtures (push) Has been cancelled
Validate frontmatter and structure / validate (push) Has been cancelled
* knowledge(style): allow event subscribers to omit trailing publisher parameters

The article told reviewers to copy the publisher signature exactly and reproduce every parameter verbatim. That contradicts events/add-new-event-parameters-at-the-end, which already states that existing subscribers bind to the leading parameters, and it produced a false positive on BCApps PR 10277 where a subscriber legitimately declared only the leading two of the publisher's three parameters.

Clarify that the name-match rule applies to every parameter the subscriber declares, state that AL binds on a leading prefix so trailing parameters may be omitted, and keep the real defect - a subscriber list that is not a prefix of the publisher's - as the anti pattern. Add the false-positive keyword for retrieval.

* fix: subscribers bind by parameter name, not by leading prefix

The first revision claimed AL binds a subscriber to a leading prefix of the publisher parameter list and that a parameter may not be skipped in the middle. That is wrong. Verified against shipping BCApps code: Test Runner - Mgt publishes OnBeforeTestMethodRun(var CurrentTestMethodLine; CodeunitID; CodeunitName; FunctionName; FunctionTestPermissions; var Skip), and ALTestRunnerResetEnvironment binds to it declaring (CodeunitID; CodeunitName; FunctionName; FunctionTestPermissions; var CurrentTestMethodLine) - omitting Skip and moving the first parameter to last. Binding is by name, so any subset in any order is valid. Detection now targets a parameter whose name or type matches nothing on the publisher.

---------

Co-authored-by: wenjiefan <wenjiefan@microsoft.com>
2026-08-28 11:07:05 +02:00
Djordje Cenic
6e321d3c56 Correct severe misconception about SetCurrentKey in the knowledge base 2026-08-22 12:04:00 +02:00
wenjiefan
ead337f9cb Address review guidance feedback
Preserve independent event seams, cover loop-carried handled state, strengthen checkpoint and UI-handler fixtures, and align DeleteAll fallback guidance.

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

Copilot-Session: 10646e50-2d8b-4cca-b02b-dfa78629e6a1
2026-08-18 14:09:11 +02:00
wenjiefan
5f1cff2fb6 Refine self-improvement review guidance
Narrow IsHandled, label-scope, UI-handler, checkpoint, and bulk-operation guidance to evidence-backed false-positive boundaries.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-08-18 11:17:14 +02:00
wenjiefan
c1057d38b2 Scope tableextension requirement to Normal fields lacking a classification
The requirement to declare DataClassification explicitly in a tableextension
applies to the Normal fields it adds; FlowFields and FlowFilters are
SystemMetadata automatically and are covered by their own article. Being
added by a table extension is also not itself a finding - the finding is a
Normal field added by a table extension that has no valid explicit
DataClassification.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-08-17 15:25:14 +02:00
wenjiefan
f8acb6cbdd Scope DataClassification inheritance to fields declared in the table
Table-level DataClassification is the effective default only for Normal
fields declared inside that table object. A tableextension cannot set the
property (AL0246) and its added fields do not inherit the base table value,
so AS0016 still requires each of them to classify itself. State this in both
privacy articles so the guidance cannot suppress genuine findings on the
tableextension pattern, which is how most partner code adds fields.

Also narrow the inheritance claim to verified AppSourceCop behaviour rather
than asserting platform-level resolution, and make the sample's table-level
default semantically representative of its fields while keeping a legitimate
field-level override and demonstrating the tableextension boundary.

Verified with alc.exe 18.0.37.11445 + Microsoft.Dynamics.Nav.AppSourceCop.dll:
the revised sample produces no AS0016, and removing the explicit
classification from the tableextension field makes AS0016 fire.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-08-17 15:14:53 +02:00
wenjiefan
a668920e96 Correct table-level data classification guidance
Document that valid table-level classifications are inherited by fields and update the privacy fixture and related guidance accordingly.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-08-17 13:16:12 +02:00
wenjiefan
6d1fada5a4 Add FP guards for field relocation to tableextension and event parameter addition
Two knowledge false-positive guards addressing bug 642303 (agent FPs on BCApps PR #9607):

- breaking-changes: relocating a field to a tableextension in the same app under the same field ID/name is a relocation, not a deletion/rename; the field still resolves on the table, so it must not be flagged as a deleted shipped field or require ObsoleteState staging. Scoped to the contract axis; silent on data migration.

- events: adding a parameter to an event publisher does not break existing subscribers (subscribers bind by name and match a subset), so the addition itself must not be reported as a breaking signature change.
2026-08-04 10:41:03 +02:00
wenjiefan
2b401aa38e Add batch-2 FP guards: sentence-case action captions + primary-key Get is transaction-cached 2026-07-21 11:08:04 +02:00
wenjiefan
c618071ea6 knowledge: add 3 FP-suppression guards from BC apps negative feedback
- upgrade/obsoletereason-need-not-restate-removal-version: ObsoleteTag carries the version; do not flag ObsoleteReason for omitting it (PR 8290)

- error-handling/unchecked-get-throws-when-record-not-found: a bare Rec.Get() errors on missing record; it is not silently ignored (PR 8584)

- performance/onaftergetcurrrecord-is-not-per-row: OnAfterGetCurrRecord fires on selection change, not per row; CalcFields there is not N+1 (PR 8617)
2026-07-21 09:45:09 +02:00
wenjiefan
89bf8fde31 tooltip: PR review flags genuinely-missing tooltips instead of deferring to AA0218
The v1.4 policy deferred every missing-tooltip case to compiler analyzer AA0218. But AA0218 severity is per-app ruleset config and is routinely downgraded to info/None or disabled, so a genuine gap can ship unflagged. PR review is the last line of defence and should raise it independently.

Keeps the false-positive guard intact: a bound field whose source table field supplies a ToolTip still inherits it and is NOT flagged. Adds the genuinely-missing case (bound field whose source is also tooltip-less, or an unbound control) as a medium-severity finding. Updates both the ui inheritance article and the style AA0218 article.
2026-07-20 14:30:29 +02:00
Wenjie Fan
1bf5a3b276
Add precision guards for systematic agent false-positive patterns (#112)
* Add precision guards for systematic agent false-positive patterns

Encodes reviewer-confirmed FP guards from the online eval: tooltip-inherited, page-trigger default return, drill-down filter not visible in diff, dual-trigger CalcFields (do.md); and a released-baseline precondition for breaking-change/upgrade findings on never-shipped symbols.

* Scope suggested-code and location to exactly the changed lines

Addresses reviewer-reported misplaced suggestions from the eval: insert-only-property emitting the whole field, single-statement rewrites anchored on the procedure name, and reductive multi-line collapses. The skill now emits a location range that matches precisely the rewritten lines.

* Correct suggested-code scoping guidance to match one-click anchor mechanics

A lone inserted line matches no existing file line and cannot be anchored; bracket the new line with one adjacent unchanged line instead. Reductive collapses omit suggested-code and fall back to a manual snippet.

* Move BC-specific FP guards out of do.md into leaf skills

do.md is the stable action-skill template and must stay domain-agnostic (JesperSchulz review). Relocate the four known false-positive patterns to their domain leaves: ToolTip-inheritance to al-ui-review; drill-down/lookup filtering and CalcFields lifecycle to al-performance-review; page-trigger exit(true) semantics to al-error-handling-review.

* Move suggested-code line-scoping guidance out of do.md into al-code-review

do.md must not carry instructions for how the review skill behaves (JesperSchulz review). Relocate the location/suggested-code precise-span rules to al-code-review's existing Suggested-code guidance section. do.md is now unchanged vs main.

* Move false-positive guards from skills into knowledge files

Keep review skills slim (finders/appliers). The FP guards and released-baseline preconditions previously embedded in leaf skills become negative-clarification knowledge articles in their domains, and the agent-findings policy edits to al-ui/al-privacy are reverted to main. Adds 6 knowledge files: error-handling (page-boolean-triggers-default-to-true), ui (bound-page-field-inherits-source-field-tooltip), performance (calcfields-in-both-getrecord-triggers-is-not-redundant, page-effective-filter-may-live-outside-the-diff), breaking-changes (unreleased-symbol-change-is-not-a-breaking-change), upgrade (unreleased-schema-change-needs-no-upgrade-path).

* Revert branch's suggested-code scoping addition in al-code-review

The three location-span shapes added to al-code-review are output-format mechanics, not domain knowledge: one-click span correctness is the engine's job (Resolve-SuggestionPlacement) and do.md already owns the suggested-code/location contract. The AL concerns the examples illustrate are already covered by existing knowledge (use-isempty-for-existence-check, data-classification-required-on-pii-fields, no-space-before-method-parenthesis). Restores al-code-review to main; the branch now adds only the 6 knowledge files.

* Restore al-ui/al-privacy review skills to base (zero diff in PR)

These two leaf skills carried an accidental net change against the PR merge-base because an earlier revert used the current origin/main (post-#110) instead of the branch base (pre-#110). Restoring them to the merge-base version removes them from the PR diff entirely. Three-way merge still preserves main's #110 suppression.

---------

Co-authored-by: wenjiefan <wenjiefan@microsoft.com>
2026-07-17 14:17:04 +02:00
Jesper Schulz-Wedde
186d8a1314
Complete AL review knowledge readiness (#108)
Some checks failed
Validate knowledge index / validate-index (push) Has been cancelled
Validate AL review fixtures / validate-review-fixtures (push) Has been cancelled
Validate frontmatter and structure / validate (push) Has been cancelled
* Complete AL review knowledge readiness

Fill telemetry and Query coverage, strengthen thin review domains, correct audited content defects, and add deterministic cheap-model evaluation and reference-integrity safeguards.

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

Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27

* Generalize review fixture discovery

Derive smoke cases from the leaf, domain, and paired-sample conventions so new leaves require no scoring-contract changes. Keep only exceptional selection/context overrides and fail when retrieval metadata cannot rank the selected article.

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

Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27

* Preserve published field IDs in sample

Keep the existing Email and Contact Email field IDs unchanged, clarify that the sample represents an independent baseline, and use a local breaking-change rule for the generic smoke evaluation.

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

Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27

* Clarify published field identity rules

State explicitly that a published field keeps its ID, name, and type while a replacement is added as a separate field under an unused ID.

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

Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27

* Align field obsoletion sample baselines

Use Email field ID 3 as the shared baseline so the bad example demonstrates a same-ID rename while the good example retains the original field and adds a separate replacement.

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

Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
2026-07-15 10:55:25 +02:00
Wenjie Fan
809af9708e
Privacy DataClassification fixes + keep Label-scope findings at minor (#102)
Some checks failed
Validate knowledge index / validate-index (push) Has been cancelled
Validate frontmatter and structure / validate (push) Has been cancelled
* Fix DataClassification default fact and broaden classification taxonomy

* Keep Label-scope findings at minor; no analyzer enforces label scope

---------

Co-authored-by: wenjiefan <wenjiefan@microsoft.com>
2026-07-14 14:23:03 +02:00
Jesper Schulz-Wedde
3d29c172a9
Promote validated community knowledge (#105)
Move eight net-new rules into the Microsoft layer, remove six overlapping articles, and update review skill discovery and references.

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

Copilot-Session: 0b130227-d418-4bc0-9e7d-ec6a37adf039

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
2026-07-14 14:20:27 +02:00
Jesper Schulz-Wedde
0bb1065bc3
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>
2026-07-14 12:53:26 +02:00
Jesper Schulz-Wedde
e0ebdd35c7
Add lifecycle error and privacy knowledge (#99)
* Add lifecycle error and privacy knowledge

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

Copilot-Session: 95c06ad8-377d-4faa-8d07-06300b1c81ec

* Fix lifecycle privacy review findings

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a26cd6d6-ff49-433e-bc53-f645c455ebdd

* Refine lifecycle privacy retrieval

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

Copilot-Session: 95c06ad8-377d-4faa-8d07-06300b1c81ec

* Make review gates explicit

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

Copilot-Session: 95c06ad8-377d-4faa-8d07-06300b1c81ec

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-07-14 12:52:56 +02:00
Jesper Schulz-Wedde
363f08f47e
Add P0 event and interface compatibility knowledge (#98)
* Add P0 extensibility compatibility knowledge

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

Copilot-Session: 645349fd-1892-48f3-8a84-db77d6abd1c3

* Correct event compatibility guidance

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

Copilot-Session: 645349fd-1892-48f3-8a84-db77d6abd1c3

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
2026-07-14 12:51:59 +02:00
Jesper Schulz-Wedde
078b869e33
Add per-row AL performance guidance (#97)
* Add per-row performance guidance

Document SetAutoCalcFields for per-row FlowFields and direct writes on iterated records, with focused reviewer retrieval cues.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 85be3fc4-5253-47b8-ba1b-6b8fd188fcea

* Bound commit checkpoints by key range

Use a capped ordered query to discover each checkpoint watermark before locking and processing only that key range.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 85be3fc4-5253-47b8-ba1b-6b8fd188fcea

* Address performance retrieval review

Retrieve Commit-in-loop guidance precisely, process exact checkpoint key lists, and narrow clone-before-write discovery.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 85be3fc4-5253-47b8-ba1b-6b8fd188fcea

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-07-14 12:51:39 +02:00
Jesper Schulz-Wedde
0e06485027
Correct performance knowledge guidance (#94)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 667c64a8-eb36-4440-bc41-6a97d8fb5542

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
2026-07-14 11:26:29 +02:00
Jesper Schulz-Wedde
5706959e4a
Fix lifecycle compatibility guidance (#93)
* Fix lifecycle compatibility guidance

Correct high-confidence Business Central guidance and samples for upgrade tags, collectible errors, trigger semantics, obsoletion, events, interfaces, API contracts, and test transactions.

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

Copilot-Session: e05a43e7-6448-4d67-9c73-798523f5d945

* Address guidance review findings

Gate SecretText guidance to BC23 and clarify that the collectible-error sample intentionally emits a message-only blocking aggregate.

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

Copilot-Session: e05a43e7-6448-4d67-9c73-798523f5d945

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
2026-07-14 11:26:16 +02:00
Jesper Schulz-Wedde
aca3986fd0
Correct security and privacy knowledge guidance (#92)
* Correct security and privacy guidance

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

Copilot-Session: 9c2eebc4-dcd5-4b85-8113-90772d818900

* Address security privacy review findings

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

Copilot-Session: 9c2eebc4-dcd5-4b85-8113-90772d818900

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
2026-07-14 11:25:03 +02:00
Jesper Schulz-Wedde
bfda67a95a
Promote security knowledge from community to Microsoft layer (#49)
* Promote security knowledge from community to Microsoft layer

Pure git-mv relocation of the SECURITY domain from the community layer to the Microsoft layer. No content changes.

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

* Address review feedback on security knowledge promotion

- do-not-grant-rights-beyond-a-users-entitlement.md: drop the See sample
  reference to a .good.al file that does not exist
- Remove the 'Contributions welcome' boilerplate line from
  compose-permission-sets, prefer-oauth2, and protect-sensitive-data
- protect-sensitive-data-in-temporary-tables: remove the pointless
  DeleteAll on the locally scoped temp buffer in the good sample and
  reword Best Practice to note local buffers are cleaned up automatically
- Drop guard-bulk-operations-with-istemporary from the promotion; it
  stays in the community layer pending a decision on whether it is security

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

* Remove Contributions welcome boilerplate from do-not-grant article for consistency

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

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-14 11:23:57 +02:00
Jesper Schulz-Wedde
766046b85d
Add data-modeling and appsource knowledge articles (MICROSOFT layer) (#65)
Author 7 remedial BCQuality knowledge articles plus good/bad AL samples
(21 files) covering AL master-table and data-model design:

- data-modeling: master No. from number series in OnInsert; use codeunit
  "No. Series" not obsolete NoSeriesManagement; setup table is a singleton;
  set Last Date Modified in OnModify and OnRename; enforce Blocked in
  referencing code not in the master.
- style: ApplicationArea required on page controls (AS0062).
- appsource: object affixes prevent collisions (AS0011).

Clean-room authored from own BC knowledge; specifics verified against public
sources only (learn.microsoft.com, microsoft/BCApps). Introduces two new
domains (data-modeling, appsource).

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-13 10:35:31 +02:00
Jesper Schulz-Wedde
34c931e1c1
Fix knowledge corpus integrity issues (#87)
Repair broken knowledge references and align review examples with canonical articles. Correct explicit version gates, restore a missing title, and recognize the plugin directory in the root guard.

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-07-13 10:34:23 +02:00
Jesper Schulz-Wedde
3aa3581f95
Add testing knowledge: UI handlers, table relations, asserterror, fixtures (P1+P2) (#62)
Some checks failed
Validate knowledge index / validate-index (push) Has been cancelled
Validate frontmatter and structure / validate (push) Has been cancelled
* Add testing knowledge: UI handlers, table relations, asserterror, fixtures (P1+P2)

Six BC-specific testing-domain knowledge articles in community/knowledge/testing/, each with .good.al/.bad.al samples:

- ui-calls-require-test-handlers
- tablerelation-requires-prerequisite-records
- handlers-enqueue-never-assert
- handlerfunctions-attribute-must-match-ui-path
- asserterror-needs-expectederror-and-code
- use-library-codeunits-for-test-fixtures

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

* Move testing knowledge from community to microsoft layer

Relocates the six P1+P2 testing-domain articles (18 files: .md + .good.al + .bad.al each) from community/knowledge/testing/ to microsoft/knowledge/testing/ per maintainer request. Pure git-mv rename; no content or frontmatter changes.

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

* Address review: merge handler articles, adopt enqueue-driven pattern

Respond to @nikolakukrika's review on #62:

- Merge ui-calls-require-test-handlers, handlerfunctions-attribute-must-match-ui-path
  and handlers-enqueue-never-assert into a single ui-handlers-in-tests article.
- Adopt the enqueue-from-test / dequeue-and-assert-in-handler pattern using
  Assert.ExpectedConfirm/ExpectedMessage (substring match), with Initialize()
  clearing LibraryVariableStorage and AssertEmpty() proving exact call counts.
- asserterror sample now uses Assert.ExpectedTestFieldError + FieldCaption instead
  of hardcoded message/code; article text points to the library helpers.
- Drop the tablerelation article and fold its test-relevant ordering point
  (relations checked on Validate/Insert(true); build parents first) into
  use-library-codeunits-for-test-fixtures.

Article count 198 -> 195.

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

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-09 10:24:51 +02:00
Jesper Schulz-Wedde
6281e7e39a
Promote ui knowledge from community to Microsoft layer (#51)
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-01 10:41:24 +02:00
Jesper Schulz-Wedde
292bdabe27
Promote performance knowledge from community to Microsoft layer (#50)
Pure git-mv relocation of all 7 performance articles from community/knowledge/performance/ to microsoft/knowledge/performance/. No content changes.

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-01 10:41:01 +02:00