Skip to content

knowledge(events): ChangeCompany leaves triggers and trigger-event subscribers running in the calling company - #152

Open
waldo (waldo1001) wants to merge 2 commits into
microsoft:mainfrom
waldo1001:community/events/changecompany-runs-triggers-in-the-calling-company
Open

knowledge(events): ChangeCompany leaves triggers and trigger-event subscribers running in the calling company#152
waldo (waldo1001) wants to merge 2 commits into
microsoft:mainfrom
waldo1001:community/events/changecompany-runs-triggers-in-the-calling-company

Conversation

@waldo1001

Copy link
Copy Markdown
Contributor

What

Adds one community article to the events domain, community/knowledge/events/changecompany-runs-triggers-in-the-calling-company.md, with .good.al and .bad.al samples. The fact: ChangeCompany redirects only the data access of a record variable. Triggers, field validation, and the database trigger-event subscribers keep running in the calling company. Switching RunTrigger off 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 (StartSession with 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 ChangeCompany moves the trigger context, two knew it does not, switched RunTrigger off, hand-copied the trigger logic, and left the subscriber double-counting in the calling company. None of the five reached StartSession. 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.md is the only article mentioning ChangeCompany. 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.md covers when to pass false; it does not discuss trigger events firing regardless, nor cross-company writes.
  • No article mentions StartSession, cross-company, or Insert(false) in this sense.
  • In-flight: open PRs searched for 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

Layer and retrieval

Community layer. microsoft/skills/review/al-events-review.md already sources the events domain across layers, so no skill change is needed; review fixtures untouched.

Scope

  • The detection signal is scoped to writes after ChangeCompany(<name>) on tables the extension does not own, or whose triggers read company data, or whose subscribers do not exit on RunTrigger = false. Reads, hand-off writes into owned trigger-free tables, and the parameterless ChangeCompany() reset are named as not flagged.
  • Left out: the per-row cache cost (already covered), SessionSettings company switching (client-side, different concern), and Job Queue or Task Scheduler as alternatives to StartSession (not needed for the fact; happy to add if wanted).
  • The samples use a custom table and subscriber rather than a base-application table so every claim in a sample comment is demonstrated by the sample itself.

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 --search for 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

  • Frontmatter has exactly the six required keys; domain matches the folder
  • ## Description present; no fenced code blocks; under 100 lines; one concern
  • Samples referenced by filename from the article and present next to it
  • validate_frontmatter.py, Test-KnowledgeIndex.ps1, Test-ReviewFixtures.ps1 pass locally
  • Only community/ is touched
  • Commit author is linked to the GitHub account; branch rebased on upstream/main

🤖 Generated with Claude Code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

waldo (waldo1001) and others added 2 commits September 11, 2026 19:43
…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>
@waldo1001
waldo (waldo1001) force-pushed the community/events/changecompany-runs-triggers-in-the-calling-company branch from eaef922 to 1e87380 Compare September 11, 2026 17:49
@waldo1001

Copy link
Copy Markdown
Contributor Author

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 ChangeCompany/StartSession/cross-company/RunTrigger relevance tokens and a deterministic cue into microsoft/skills/review/al-events-review.md): agreed this article needs it to be reliably surfaced, but that file lives under microsoft/skills/, which is maintainer-reviewed territory separate from this community/ contribution. I'd rather not bundle a skill-mechanics change into a knowledge-content PR. I'll follow up with a dedicated PR for the skill wiring — happy to open it now or wait for a maintainer nod on scope, whichever you'd prefer.

Summary of this update:

  • Best Practice now states StartSession's async/fire-and-forget semantics (Ok = session started, not "codeunit succeeded"; no transaction extension; errors don't propagate) and scopes the recommendation accordingly, with two new Learn citations.
  • changecompany-runs-triggers-in-the-calling-company.good.al: replaced the FindLast()+1 entry-number pattern with AutoIncrement = true, which the platform guarantees is race-free across concurrent transactions.
  • Rebased onto upstream/main; picked up the sample-link convention change (now [slug.al](slug.al)) and an upstream promotion of the overlap-neighbour article (communitymicrosoft), both reflected in the article and in local validation evidence.

waldo (waldo1001) added a commit to waldo1001/BCQuality that referenced this pull request Sep 11, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants