knowledge(events): ChangeCompany leaves triggers and trigger-event subscribers running in the calling company - #152
Conversation
Jesper Schulz-Wedde (JesperSchulz)
left a comment
There was a problem hiding this comment.
This article captures a real and important platform behavior. The ChangeCompany trigger-context statement matches Microsoft Learn, the RunTrigger = false subscriber caveat is well motivated, the scope exclusions are thoughtful, and the exact PR tree passes the frontmatter, index, fixture, and whitespace validators.
I found three integration/correctness issues that should be addressed before this becomes authoritative agent guidance. Two are inline. The third is discoverability: al-events-review.md currently extracts event-design tokens such as EventSubscriber and OnAfter, but not ChangeCompany, StartSession, cross-company writes, or RunTrigger database operations, and it has no targeted cue for this article. The anti-pattern can therefore be missed when the changed file only performs ChangeCompany followed by Insert/Modify/Delete/Validate and the relevant table/subscribers live elsewhere. Please add relevance tokens and a deterministic cue for the article's actual detection signal.
With the asynchronous-semantics qualification, a concurrency-safe sample key, and review-skill wiring, this should be a strong addition.
|
|
||
| ## Best Practice | ||
|
|
||
| Use `ChangeCompany` to read. Access rights in the target company are still enforced, so reads are safe. When the goal is business data in another company, run the code in that company: `StartSession` takes a company name and runs a codeunit there, so triggers, validation, and subscribers all execute with the target company as their context. Learn notes that a background session costs as much as a user session to start, so batch the work rather than starting one session per row, or let the target company process a hand-off row on its own schedule. A direct cross-company write is acceptable only as such a hand-off into a table the writing extension owns, whose triggers do not read company data and whose trigger-event subscribers exit when `RunTrigger` is false, using `Insert(false)`, `Modify(false)`, or `Delete(false)`, and never `Validate`. |
There was a problem hiding this comment.
StartSession does run the codeunit in the target company, but it is an asynchronous background session, not a transparent replacement for a synchronous business write. The caller only learns whether the session started; target-operation errors do not propagate back, the work is outside the caller's transaction, and completion/results require explicit tracking. Please state those semantics and limit this recommendation to operations that may safely be queued/fire-and-forget (or add a durable hand-off/status/error channel). For a write whose success must be known before the caller continues, this sample currently changes the business contract rather than merely correcting company context.
There was a problem hiding this comment.
Fixed. The Best Practice section now states StartSession's async/fire-and-forget semantics explicitly: the Ok return value reports only whether the background session started, not whether the codeunit's work inside it succeeded; the caller's transaction does not extend into it; and an error raised there does not come back to the caller (it has to be logged or telemetered from inside that session, per the AL error-handling docs). The recommendation is now scoped to work the caller does not need to confirm synchronously, with an explicit call-out that a write whose success must be known needs a durable status/error channel rather than a bare StartSession call.
Added citations: Session.StartSession Return Value, and the AL error handling doc's guidance on logging errors from inside a rolled-back transaction.
| var | ||
| LastRequest: Record "Transfer Request Good"; | ||
| begin | ||
| if LastRequest.FindLast() then |
There was a problem hiding this comment.
FindLast() + 1 is not concurrency-safe. Two callers can start target-company sessions concurrently, both calculate the same next entry number, and one background session then fails with a duplicate key—without that failure reaching the initiating caller. Please use a concurrency-safe key strategy (for example an AutoIncrement entry number or another platform-backed sequence) so the “good” fixture remains reliable under the asynchronous pattern it recommends.
There was a problem hiding this comment.
Fixed. NextEntryNo() (FindLast()+1) is removed; "Entry No." is now AutoIncrement = true, which the platform guarantees assigns a different number to each concurrent transaction (Learn: "if several transactions are performed at the same time, they will each be assigned a different number"), so concurrent background sessions in TargetCompany no longer race on the same value.
…bscribers 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>
…rrency-unsafe sample key Addresses PR microsoft#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>
eaef922 to
1e87380
Compare
|
Thanks for the review — pushed a fix for both inline points (replies above), also rebased onto current upstream/main. On the third point (discoverability — wiring Summary of this update:
|
…in-the-calling-company PR microsoft#152 review (JesperSchulz) requested StartSession async-semantics qualification and a concurrency-safe sample key; records the fix, the overlap-neighbour path update after its community->microsoft promotion upstream, and the rebase note (directoryRenames heuristic gotcha). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
What
Adds one community article to the
eventsdomain,community/knowledge/events/changecompany-runs-triggers-in-the-calling-company.md, with.good.aland.bad.alsamples. The fact:ChangeCompanyredirects only the data access of a record variable. Triggers, field validation, and the database trigger-event subscribers keep running in the calling company. SwitchingRunTriggeroff skips the trigger code but not the subscribers, because the runtime raises the trigger events on every database operation and only passes the flag along. The Best Practice is to run the code in the target company (StartSessionwith the company parameter) and to keep direct cross-company writes for trigger-free hand-off tables the extension owns.Why this is a knowledge file (admission test)
Generation is where models fail. In five generation runs of the same task (create a row in another company whose OnInsert reads setup and whose OnAfterInsertEvent subscriber maintains a counter), four wrote to the other company with side effects landing in the calling company: two assumed
ChangeCompanymoves the trigger context, two knew it does not, switchedRunTriggeroff, hand-copied the trigger logic, and left the subscriber double-counting in the calling company. None of the five reachedStartSession. On review, capable models do catch the defect when the trigger and subscriber are in the same file; the evidence section records that honestly.Overlap check
community/knowledge/performance/changecompany-in-loop-drops-caches.mdis the only article mentioningChangeCompany. It covers the per-row cache cost of the call and says nothing about execution context, triggers,RunTrigger, or subscribers. Delta; cross-referenced from the Description.microsoft/knowledge/performance/pass-false-to-insert-when-trigger-not-needed.mdcovers when to passfalse; it does not discuss trigger events firing regardless, nor cross-company writes.StartSession,cross-company, orInsert(false)in this sense.changecompany,startsession,cross-company,multi-company. Only Add Job Queue reliability and scheduling guidance #148 (Job Queue reliability) matches a keyword; it does not state this fact.Sources
bc-version: [all]: ChangeCompany, Insert(Boolean), the table trigger events and StartSession(var Integer, Integer, Text, var Record) are all runtime 1.0 on their Learn pages.Layer and retrieval
Community layer.
microsoft/skills/review/al-events-review.mdalready sources theeventsdomain across layers, so no skill change is needed; review fixtures untouched.Scope
ChangeCompany(<name>)on tables the extension does not own, or whose triggers read company data, or whose subscribers do not exit onRunTrigger = false. Reads, hand-off writes into owned trigger-free tables, and the parameterlessChangeCompany()reset are named as not flagged.SessionSettingscompany switching (client-side, different concern), and Job Queue or Task Scheduler as alternatives toStartSession(not needed for the fact; happy to add if wanted).Evidence
changecompany-runs-triggers-in-the-calling-company
overlap: community/knowledge/performance/changecompany-in-loop-drops-caches.md — delta — neighbour only states "ChangeCompany retargets a record variable to another company's data and drops the in-memory caches"; nothing about triggers, RunTrigger, Validate or subscribers running in the calling company. Cross-referenced from the Description.
in-flight: none —
gh pr list --state open --searchfor changecompany, startsession, cross-company, multi-company returns only PR #148 (Job Queue reliability; adjacent, not the same fact)claim: triggers still run in the current company after ChangeCompany — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-changecompany-method (Remarks)
claim: access rights in the target company are respected — same page (Remarks)
claim: ChangeCompany() without a name changes back to the current company — same page (Parameters)
claim: Insert(false) / Insert() skips the OnInsert code; RunTrigger defaults to false — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-insert-boolean-method and https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-delete-method
claim: database trigger events are raised by the runtime on every database operation and pass RunTrigger to the subscriber — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-event-types (Database trigger events; order of execution) and https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/triggers-auto/events/table/devenv-onafterinsertevent-table-trigger (RunTrigger parameter)
claim: StartSession takes a Company parameter and runs the codeunit in that company; a background session costs as much as a user session to start — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/session/session-startsession-integer-integer-string-table-method (Parameters, Remarks)
claim (bad sample comment): the OnAfterInsertEvent subscriber fires on Insert(false) in the calling company — inference from the two Learn statements above (events raised on every database operation; triggers run in the current company); no observed run available in this session
bc-version: [all] — ChangeCompany, Insert(Boolean), StartSession(var Integer, Integer, Text, var Record) and the table trigger events are all runtime 1.0 per their Learn pages
precision: Insert(false)/Modify(false)/Delete(false) into an owned hand-off table whose triggers do not read company data and whose subscribers exit on RunTrigger = false; reads after ChangeCompany; ChangeCompany() reset — carved out in Anti Pattern (Detection signal paragraph) and Best Practice
cold review (bad sample, Fable-class default model): caught — "OnAfterInsertEvent is raised even when Insert(false) is used (that is why RunTrigger is a parameter) ... increments Open Requests in the calling company while the request row landed in TargetCompany"
cold review (bad sample, Sonnet): caught — "the subscriber fires unconditionally regardless of RunTrigger/Insert(false) ... ChangeCompany only affects the record variable it's called on, not variables touched inside triggers/events it raises"
cold review (first draft, Insert(true) shape, Fable-class): caught — same reason; also found a real sample bug (FindLast on the Init'ed variable), fixed
generation test (Fable-class): no defect — used StartSession(company) and explained why ChangeCompany was avoided
generation test (Sonnet, run 1): defect — ChangeCompany + Insert(false) + hand-copied setup + manual counter; ignored that the OnAfterInsertEvent subscriber still fires in the calling company (double count). This is the shape of the bad sample.
generation test (Sonnet, run 2): defect, wrong premise — commented "ChangeCompany suppresses the target table's OnInsert trigger (and therefore the OnAfterInsertEvent subscriber)"; Insert(false) + manual counter, double count in the calling company
generation test (Sonnet, run 3, reworded prompt): defect, knowingly — described both facts correctly, then shipped ChangeCompany + Insert(false) + manual counter and called the calling-company side effect "unavoidable from the caller side"; did not consider StartSession
generation test (Haiku): defect, wrong premise — "ChangeCompany switches context so the target company's OnInsert trigger applies its default location"; ChangeCompany + Insert(true)
generation summary: 4 of 5 runs wrote to the other company with side effects in the calling company; 0 of 5 non-Fable runs reached the Best Practice (StartSession with the company parameter); only the Fable-class run avoided the defect
warm review (bad sample): flagged citing community/knowledge/events/changecompany-runs-triggers-in-the-calling-company.md (line 20, double count in the calling company)
warm review (good sample): clean
script: 0 error(s), 0 warning(s) (bcq-validate.sh, 2026-09-03; upstream validator, knowledge index, review fixtures, contributor checks, overlap all green)
verdict: ready, with a stated caveat — deterministic checks and warm review pass; the admission case is generation-side (4 of 5 runs produce the defect, none reach the remedy) while capable models catch the defect on review. Say so in the PR body.
Checklist
domainmatches the folder## Descriptionpresent; no fenced code blocks; under 100 lines; one concernvalidate_frontmatter.py,Test-KnowledgeIndex.ps1,Test-ReviewFixtures.ps1pass locallycommunity/is touchedupstream/main🤖 Generated with Claude Code