al-performance-review.md's tokens/cues only recognized Report.RunModal,
so a failing Modify(); Report.Run(..., true, ...) path was never
worklisted even though Report.Run shares the exact same RequestWindow-
blocking-dialog mechanism as Report.RunModal (they differ only in
whether the report instance is cleared afterward). Added Report.Run to
the token list and the targeted cue, with the same request-page-
suppressed exclusion, and extended the knowledge article's Description
bullet to name both methods explicitly instead of only RunModal.
XmlPort has no RunModal method (static or instance) - Microsoft Learn
confirms only Xmlport.Run(Integer [, Boolean RequestWindow] [, Boolean]
[, var Record]). Replaces the invented API with the real one throughout
the article, worklist cue, and token list, and explains the platform
error message's own "XmlPort.RunModal" wording as the same kind of
legacy phrasing already noted for Form.RunModal/RequestForm.
Also corrects UseRequestPage(false): that's a Report instance method
only, not applicable to XMLport, which uses the UseRequestPage = false
object property or Run's RequestWindow argument instead.
Fixes the two sample references to use the READ-convention markdown
link form (Test-KnowledgeIndex.ps1's Knowledge-Retrieval.ps1 check was
failing on plain backtick text).
Rebased onto upstream/main to resolve conflicts with #148's job-queue
additions to al-performance-review.md - both sets of worklist tokens
and cues are retained.
Adds Page.RunModal / Report.RunModal / XmlPort.RunModal / UseRequestPage
to the extracted-token list and one deterministic worklist cue with
exclusions, so the article is selected from the RunModal call itself
rather than only via a co-located Commit/Modify token. Codeunit.Run in
that position is routed to its existing owner article.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
"AL methods limited during write transactions: commit before RunModal
and Codeunit.Run" - so a developer or agent searching for the runtime
error text lands on the one article that covers all four restricted
methods. Slug and sample stems renamed to match; keywords gain the
error's own phrase and the legacy Form.RunModal name it still uses.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Mirrors the structure of the platform's own error message so the
Report.RunModal and XmlPort.RunModal request-page exceptions are
visible at a glance instead of buried in prose.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Message runs asynchronously - it is queued and shown when the calling
method ends or another method requests input - so it never pauses the
transaction and holds no lock. Only Confirm and StrMenu wait for the
user.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A modal page does not behave like Confirm/StrMenu inside a write
transaction: the platform refuses Page.RunModal (and Report/XmlPort
.RunModal with a request page, and Codeunit.Run with its return value
used) with a runtime error instead of holding the lock. The new article
documents that guard - verified against Microsoft Learn (Codeunit.Run
transaction semantics), microsoft/AL#5452, Microsoft's own Base
Application (Commit(); Page.RunModal pattern), and a live reproduction
on Business Central 26 quoted verbatim. avoid-user-prompts-inside-
transactions.md keeps its scope to the prompts the platform does allow;
"modal page" is removed from its list because that case is refused, not
stalled.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Add Finance posting domain-knowledge pilot
Adds three atomic domain-rule knowledge files under community/knowledge/finance/ as a pilot for Type-A (normative) Business Central domain knowledge: post through the posting engine, treat posted ledger entries as immutable, and treat the Dimension Set ID as the source of truth for dimensions. Includes a good/bad AL sample pair for the posting rule.
These encode BC-specific invariants that LLMs reliably get wrong, fitting the existing remedial/atomic knowledge grain with no schema or contract changes. Passes the repo frontmatter validator and is discovered by the knowledge index.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Clarify editable operational fields on posted ledger entries
Addresses review feedback from @JeremyVyska on PR #57: the immutability rule applies to financial content, not the whole entry. Reframes the Description around financial content and gives the operational-field exception (payment/application data, on-hold, applies-to, communication fields edited via CustEntry-Edit/VendEntry-Edit and the ledger entry pages) its own paragraph in Best Practice instead of understating it as a narrow set.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Expand Finance pilot into a source-verified review domain
Move Finance knowledge to the Microsoft-owned layer, add nine scoped rules with eighteen AL samples, and register bounded Finance review with complete paired evaluation coverage.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Fix Finance review applicability and ownership boundaries
Remove application-area gating and later VAT-field dependencies, align dynamic shared conventions, separate SCM ownership, and keep journal examples focused on the intended invariant.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Remove logo branding (#194)
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Add title and description to README
* Add foundational AL developer knowledge (#195)
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Clarify locale-safe DateFormula Evaluate inputs (#193)
* Clarify locale-safe DateFormula Evaluate inputs
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Normalize DateFormula article sections
Keep the analyzer-gap explanation in Description and its scoped probe evidence in References, without a novel Validation section. Normative guidance and fixtures are unchanged.
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>
* Add SCM functional knowledge domain (#192)
* Add SCM functional knowledge domain
Introduce nine source-backed rules with original AL sample pairs, bounded SCM review routing, and complete positive/clean evaluation coverage.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Normalize SCM knowledge and review ownership
Align article and AL sample conventions, keep BC facts separate from review mechanics, and clarify reciprocal Finance ownership without bespoke shared test assertions.
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>
---------
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
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>
* 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>
Generate a deterministic action-skill index from frontmatter, publish structural schemas for orchestration and findings, and validate flat review composition in CI.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: dayland <dayland@microsoft.com>
Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
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
* Add bounded knowledge retrieval
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0d8b7764-f15a-49ea-8d50-d9147334f8af
* Fix bc-version overflow and pathless-row identity in bounded retrieval
Catalog matching compared an Int32 -BCVersion against a bigint range bound.
PowerShell coerces the right operand to the left operand's type, so a bound
wider than Int32 threw a conversion error and failed the whole domain catalog
rather than the single row. Metadata validation already accepts such bounds,
so compare as bigint on both sides.
The shared pager built its oversized-row message with $row.path, which
throws under Set-StrictMode -Version Latest when a row carries no path,
replacing the explicit bound failure with a property-lookup error. Resolve the
path defensively for dictionary and object rows so the offset-based fallback
is reachable.
Both paths gain regression coverage that fails without these fixes.
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>
Copilot-Session: 0d8b7764-f15a-49ea-8d50-d9147334f8af
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>
Remove historical naming and external orchestrator references, and present the app-folder review as one example of the broader host-native skill pattern.
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Document standalone review runner contract
Keep model selection and scheduling outside BCQuality while allowing orchestrators to run isolated review leaves concurrently with deterministic rollup semantics.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Support complete app folder reviews
Define folder-path as a current-state review scope and accept it across the standalone adapter, broad coordinator, and every AL review leaf.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Add walk-up app review quick start
Put the complete app-folder installation and prompt flow directly in the README so partners can discover the standalone experience without reading integration details first.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Organize conceptual guides under docs
Move architecture and standalone runner documentation out of the repository root, add a documentation index, and update all inbound links.
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>
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
* Simplify standalone AL review skill
Rename the host-facing skill to al-code-review, reduce it to a thin Entry adapter, document the architecture, and validate host skill metadata.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: af96bb3d-893a-48a2-8298-c8f4271c162c
* Anchor plugin paths at PLUGIN_ROOT and restore the layer-filter caveat
The adapter delegated index preparation to Entry's Preparation step, but that
step is written for the clone model: it runs `pwsh ./tools/Build-KnowledgeIndex.ps1`
from the checkout root. A plugin host's working directory is the user's own
project, so the path does not resolve and the index is never built. Because
knowledge-index.json is gitignored, a fresh install has none, and READ silently
degrades to path-based discovery. The adapter now resolves Entry's repo-relative
paths against PLUGIN_ROOT and names the absolute index build; the generator
resolves its own root, so it indexes and writes the right tree from any cwd.
Entry also asserted that pruning has always happened before it runs, which is
false for an installation that ships the whole tree. Entry now scopes that
guarantee to consumers that actually prune, and the caveat dropped in the
rewrite - that enabled-layers narrows discovery rather than denying access - is
restored in the adapter and summarized in the README.
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: af96bb3d-893a-48a2-8298-c8f4271c162c
Copilot-Session: 0b67b90d-e4b4-4b92-9684-726c72c43b3f
What an API page publishes for an enum field depends on the OData schema
version: member names under 2.0, the caption as Edm.String under 1.0, never
the ordinal. Custom APIs defaulted to 1.0 through BC 23 and to 2.0 from BC 24.
LLMs assert one carrier as universal and get one direction wrong.
Co-authored-by: waldo1001 <12088142+waldo1001@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
* 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
* feat(community/agents): add AL agent quality guidance
- add 20 agent knowledge rules with good and bad AL samples
- clarify setup dialog shape, temporary persistence, permissions, profiles, instructions, capability registration, and interface wiring
- add the community-owned AL agents review skill
- make review fixture discovery layer-aware with custom, community, and Microsoft precedence
- document layer-aware evaluation behavior
* fix(community/agents): align setup and permission samples
- mark agent setup pages as non-extensible where required
- narrow the agent profile by hiding an unrelated sales-order field
- define a dedicated read-only permission set for the sales review agent
- assign AL-defined permission sets with system scope and the owning app ID
- clarify the permission scope guidance for default access controls
* Address agent review feedback
* ShowMandatory + OnQueryClosePage Check
* Tighten mandatory-field review guidance
Require explicit ShowMandatory in the good sample, acknowledge that NotBlank marking is unreliable, and limit findings to visible editable controls on paths where users must supply a value.
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>
Copilot-Session: 89dba8c8-6529-4b60-956f-875a59be499d
* Add community knowledge on AL boolean operators not short-circuiting
AL gives no short-circuit (lazy) evaluation guarantee for and/or/xor —
neither the AL operators nor the boolean operators documentation defines a
lazy evaluation order. LLMs trained on C#, JavaScript, or SQL assume the
left operand guards the right, which produces conditions where a guard
does not protect an unsafe subscript or a field read after a failed Get,
and where an expensive operand is paid on every path.
Adds community/knowledge/performance/boolean-operators-do-not-short-circuit.md
with good/bad AL companions. The guidance prefers nested if when one operand
depends on another, while keeping and/or legitimate for operands that are
independently safe and cheap, so a reviewer does not flag harmless bound
checks. This is the first article in a community performance domain; the
Microsoft performance review leaf skill already sources candidates by domain
across every enabled layer, so no skill change is needed to reach it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Add case true of pattern for long condition chains
Follow-up from PR review: nested if is the right answer for two or three
dependent conditions, but past that the nesting becomes the problem. AL's
case statement is the flat alternative — the control statements
documentation states a value set "must be an expression or a range" and
that the first matching value set executes, so case true of / case false of
accept boolean expressions and stop at the first match. That is the
laziness the boolean operators do not provide.
Adds case-true-of-for-long-condition-chains.md with good/bad AL companions:
case false of for guard chains where every condition must hold, case true of
for first-match dispatch. The bad sample shows both failure shapes — a
five-level if ladder, and the worse escape of collapsing it into an and
chain, which trades nesting for a real defect.
Cross-links both articles, and adds the threshold to the short-circuit
article's Best Practice so following it does not lead to a deep ladder.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Comma-group the pure value sets in the case false of sample
Review feedback: the five value sets sharing exit(false) should be comma
separated. Applied to the three that are pure field reads with no order
dependency.
The Get and the Blocked read keep their own value sets. The documentation
guarantees that the first matching value set executes, which orders matching
across separate value sets; it says nothing about evaluation within one
comma-separated set, and the natural lowering of that is an equality-or
chain — where AL's or does not short-circuit. Grouping the Get with the
checks that must precede it would rest the sample's correctness on
undocumented behaviour, which is the defect these two articles exist to
prevent.
Encodes the boundary in the Best Practice section so the grouping is applied
where it is safe and not where it is not, and scopes the repeated-exit
detection signal explicitly to nested chains so the case sample does not read
as its own anti-pattern.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Use a single exit(false) for the whole case false of chain
Review decision: all five conditions share one action, so they share one
comma-separated value set with a single exit(false).
Aligns the article's Best Practice with the sample — it previously told
authors to keep side-effecting conditions in their own value set, which the
sample no longer does — and drops the now-contradictory wording about listing
the failure action per condition.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Drop the parentheses from the case false of value sets
Applies NKarolak's review suggestion: a case value set needs no parentheses
around a comparison.
Encodes the rationale in the Best Practice section, since it is a real
advantage of the pattern and is not documented elsewhere in the repo. The AL
operator hierarchy places and/xor above the comparison operators and or just
above them too, so parentheses are mandatory in an and chain — A = B and
C = D misparses without them — while a case value set has no and to bind
tighter and needs none. That inverts the precedence most developers arrive
with from C#.
Also notes in the bad sample that its parentheses are not optional, so the
two samples contrast on parentheses as well as on evaluation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Address correctness review: or/xor pattern, case value-set ordering
Fixes two points from JesperSchulz's review on PR #136.
1. boolean-operators-do-not-short-circuit.md gave one fix — nested if — for
and, or, and xor alike. That's only correct for and: nesting if A then if
B then Action drops the A-true/B-false case of A or B, silently changing
the result. Gives or its own early-exit pattern (if A then exit(true);
exit(B)), warns explicitly against applying the and-rewrite to or, and
clarifies that xor is not a short-circuit candidate in any language since
its result always depends on both operands. Adds an or fixture
(IsEligibleForFreeShipping) to both samples so an agent has a concrete
pattern to match instead of extrapolating from the and-only examples.
2. case-true-of-for-long-condition-chains.md derived stop-at-first-match for
one comma-separated value set from the documentation's guarantee about
the first matching value set — plural, i.e. ordering across value sets,
which is not the same claim. The good sample's Item.Get / Blocked pair
depended on the one the docs don't make. Restructures the sample to only
comma-group the three pure, order-independent checks; Get and Blocked
keep their own value sets, in order, relying solely on the guarantee that
is actually documented. Description and Anti Pattern now state that
boundary so it isn't re-collapsed later.
Also carries forward a parenthesis fix (not Item.Blocked in the collapsed
bad sample) that was made two commits ago but never landed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* 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
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.
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>
