Skip to content

Fix Expense Agent mailbox cleanup and communication scheduling - #11615

Draft
Prangshuman Das (t-prda) wants to merge 14 commits into
mainfrom
t-prda-expense-email-lifecycle-pr
Draft

Prangshuman Das (t-prda) wants to merge 14 commits into
mainfrom
t-prda-expense-email-lifecycle-pr

Conversation

@t-prda

@t-prda Prangshuman Das (t-prda) commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

What & why

This change fixes the Expense Agent email lifecycle so incoming receipt processing and outgoing communication remain independent.

  • Clearing an incoming account clears only its identity and folder data while preserving the receipts preference and all outgoing settings.
  • Clearing the no-reply account preserves outgoing preferences and the incoming configuration.
  • Channel readiness is based on the saved preference plus the actual registered account ID and connector, without treating transient mailbox connectivity failures as account deletion.
  • Missing account registrations are repaired by the existing setup-wizard and scheduler reconciliation paths. The PR does not subscribe to native email-account deletion; this follows the simpler Sales Order Agent and Payables Agent pattern.
  • The dispatcher skips only unavailable channels, so outgoing delivery continues without an incoming mailbox and incoming receipt processing continues without a no-reply account.
  • Common scheduler, dispatcher completion, error-handler, recovery, and setup-save paths reconcile against the latest persisted setup. When no channel is runnable, pending dispatcher and recovery work is cancelled without clearing IDs for tasks that could already be running.
  • Wizard edits remain staged: Cancel does not persist changes or reconcile tasks, while a successful save reconciles once from the persisted configuration.
  • The obsolete hidden mailbox and no-reply AssistEdit handlers were removed from the legacy setup card; the active configuration wizard and table AssistEdit paths remain unchanged.

The updated AB#644162 repro is authoritative: clearing the incoming account must not disable the receipts preference. Some older work-item wording describes different behavior.

PR #11541 is already merged into main; this branch contains only the follow-up commits rebased onto current main.

Linked work

Fixes AB#644162
https://dev.azure.com/dynamicssmb2/Dynamics%20SMB/_workitems/edit/644162

Fixes AB#644157
https://dev.azure.com/dynamicssmb2/Dynamics%20SMB/_workitems/edit/644157

Fixes AB#632913
https://dev.azure.com/dynamicssmb2/Dynamics%20SMB/_workitems/edit/632913

How I validated this

  • I read the full diff and it contains only changes I intended.
  • I built the affected app(s) locally with no new analyzer warnings in the touched files.
  • I ran an earlier version of the change in Business Central and confirmed the mocked lifecycle behavior; current-head runtime limits are stated below.
  • I added or updated tests for the new behavior.

Build validation

  • Current head 881b9a99ebd24b347e9c7d172d90c6425f3e7f7b: after merging current main, the Expense Agent app and test app passed Default and the full current Clean symbol set (CLEAN25 through CLEAN30) with AL compiler 18.0.41.8481, the repository ruleset, CodeCop, and UICop on Default. The resolved wizard and lifecycle test file produced no diagnostics; only pre-existing warnings elsewhere remain. Standalone AppSourceCop and PerTenantExtensionCop runs do not reproduce BCApps first-party CI configuration, so the completed pipeline is authoritative for those analyzers.
  • The dispatcher fixture uses the existing Copilot Test Library and Privacy Notice APIs to activate the product-app capability plus Azure OpenAI and Expense Agent approvals inside Codeunit isolation. Those shared records, including commits made by the exercised production flow, are rolled back at the suite boundary; the fixture does not alter an external tenant, delete privacy rows, or bypass production capability checks.
  • Method-level setup removes only native outbox and sent-email records owned by prior Expense communication mock accounts before the connector library deletes those test accounts. Native deletion triggers remove the associated failed-email errors and message data. Unrelated application outbox rows are neither required to be absent nor deleted, and the foreground-send observer validates only the current random fixture account plus its recorded failed attempts.
  • The test codeunit no longer declares explicit Email Outbox or Sent Email permissions. TestPermissions = Disabled runs these normal CI tests in the runner's SUPER context; the header was redundant and did not grant permissions to product users. RequiredTestIsolation = Codeunit remains separately necessary as the standard-CI-selected suite rollback boundary for persistent fixture writes and explicit commits; it is not required by HTTP mocking.
  • The non-persisting repair API is named RepairMissingEmailAccountsInBuffer; the Boolean overload retains optional persistence. Scheduler reconciliation is named ReconcileCommunicationScheduling, and the internal dispatcher pass is named ProcessIncomingAndOutgoingEmails. These are naming changes only.
  • The wizard saves with Modify(false) without automatic scheduling, then ApplyScheduleChange reconciles once after defaults. Missing registrations remain repaired by scheduler persistence and staged wizard buffer repair.
  • Hardcoded company-name allowlists were removed from all four lifecycle suites. Dispatcher and Welcome retain on-prem/SaaS-infrastructure safeguards, outbound HTTP blocking, scoped fixture resets, unique record identities, and no real Task Scheduler operations.
  • Dispatcher and Welcome use RequiredTestIsolation = Codeunit. The repository test selector includes None|Codeunit in standard passes, whereas their previous Function setting was excluded; both use the default UnitTest test type and are selected in the Default pass. Their setup resets owned persistent state per method, and codeunit isolation rolls back explicit commits at the suite boundary.
  • The focused inventory is 68 tests: Scheduling 6, Mailbox 27, Dispatcher 22, and Welcome 13. All 26 tests added by this PR (4 scheduling, 13 mailbox, and 9 dispatcher tests) retain specific Scenario/Given/When/Then comments; the 42 pre-existing tests remain. The removed tests covered missing setup before endpoint resolution and saved Canary selection; the URL override seam remains for the other mocked HTTP tests so they never fall through to Key Vault.
  • The wording follows the user-requested convention and the principles in the BCQuality AL testing guidance; this is not a claim of full BCQuality certification or a full automated review.

CI and runtime validation

CI run 35601417644 completed successfully on historical head 8971b4308156a526e8fb1d9003be4fd00359f161. Its W1 Default XML contained 2,750 tests with 0 failures, 0 errors, and 0 skips; W1 IntegrationTests contained 2,697 tests with 0 failures, 0 errors, and one unrelated tolerated Subcontracting skip. On that head, Scheduling 6/6, Mailbox 32/32, and the three existing System Application email-account deletion regressions passed. Dispatcher and Welcome were absent because their previous Function isolation was not selected by those standard result categories.

That successful run and run 35660241576 are historical evidence for earlier heads. CI run 35669365249 on head 26d64b9ddb233f2fd5771fc0a323d350719e18f0 first made Dispatcher and Welcome discoverable: Scheduling 6/6, Mailbox 27/27, Welcome 13/13, and 14/24 Dispatcher tests passed in W1 Default. The other 10 Dispatcher tests all failed at the same missing capability/privacy fixture assertion. W1 IntegrationTests had one unrelated Subcontracting arithmetic failure; the identical test was an unrelated tolerated skip in the prior green run.

CI run 35688598743 on head 90c49725e2e42d0f236ce5a3fce22b540f3bef7e confirmed the capability fixture: Scheduling 6/6, Mailbox 27/27, Welcome 13/13, and 14/24 Dispatcher tests passed, for 60/70 lifecycle tests. All 10 Dispatcher failures occurred before their test bodies at one global native Email Outbox.IsEmpty prerequisite; the first Dispatcher method already encountered unrelated outbox data. Head e63c7b2d73cd45307b44c47c2e7168bcb7ad732c replaced that global assumption with fixture-account-scoped cleanup and assertions; CI run 35709999459 was cancelled after that head was superseded.

CI run 35759345466 completed successfully on exact current head 881b9a99ebd24b347e9c7d172d90c6425f3e7f7b. The W1 Default XML proves that all 68 focused lifecycle methods executed and passed with no failures, errors, or skips: Scheduling 6/6, Mailbox 27/27, Dispatcher 22/22, and Welcome 13/13. The System Application Email Accounts Test methods DeleteNonDefaultAccountTest, DeleteDefaultAccountPromptNewAccountCancelTest, and DeleteDefaultAccountPromptNewAccountTest also executed and passed. The removed MissingSetupSkipsEndpointOverrideAndHttp and SavedCanarySelectionReachesCommunicationEndpoint methods are absent from the result XML. This is runtime evidence from the CI artifact, not an inference from green check status. The pipeline is not described as warning-free.

Before the CI-warning, portability, deletion-hook-removal, and isolation-discovery follow-ups, an earlier Default test app was published with Synchronize to the local NAV instance and the then-75-test suite passed sequentially (6 scheduling, 32 mailbox, 24 dispatcher, 13 welcome). That historical suite included five immediate-deletion tests now removed and used different test-isolation metadata; it is not claimed as a current-head 68/68 run.

The local NAV extension endpoint later stalled during synchronization and its extension state remains blocked/unknown. No current-head package was published and no local runtime tests, package probes, service resets, cancellations, or server changes were performed.

The scheduling matrix covers 64 account-registration and preference combinations, while focused regressions retain the independent-channel and disabled-agent cases. The tests do not create, cancel, query, or assert real Task Scheduler tasks. Dispatcher tests invoke the production synchronous incoming/outgoing pass with native email connector and HTTP mocks.

The test app packages four mocked HTTP response fixtures. Native TestHttpRequestMessage exposes request routing but not headers or body. A read-only pre-auth observer therefore verifies multipart UTF-8 serialization, intended user/correlation headers, and that authorization has not yet been attached; this is supporting HTTP contract coverage rather than a requirement of the three core bugs. The communication URL test seam remains Text because SecretText parameters are prohibited on subscribed events by AL0795; the real secret URL never flows through that event.

For receipt tests, an internal attachment-source event supplies temporary attachment BLOBs because attachment Media import was unavailable in the tested fixture. The tests still exercise inbox retrieval, Expense Agent email persistence, mark-as-read behavior, multipart construction and UTF-8 content, HTTP status handling, callback/outbox correlation, and mocked connector delivery.

Account-selector cancellation and clear-confirmation regressions are covered directly, but the focused suite does not drive a full setup-wizard TestPage Cancel flow. HTTP 200 response-body failure semantics are intentionally not asserted or changed by this PR.

Risk & compatibility

  • No schema changes, production permission-set changes, broader user grants, or email-account deletion subscriber.
  • Missing registrations are cleared during existing wizard/scheduler reconciliation rather than synchronously inside native account deletion.
  • No access-control, Entra application, Canary policy, or unrelated service-contract changes.
  • Account availability uses local email account registration rather than Graph connectivity, so transient authorization or network failures remain operational errors and do not erase preferences.
  • Failed scheduled-task cancellation retains the stored task ID to avoid creating a duplicate successor while an existing task may be running.
  • Real Task Scheduler timing/concurrency and live SaaS/Outlook delivery were not exercised locally. The PR should remain draft until the manual SaaS scenarios are completed.

Relates to AB#644162, AB#644157, AB#632913. Parent fork: 3fc2115 (PR #11541 unchanged).

Production Default build passed with analyzers. An earlier full CLEAN build passed before the final endpoint seam; latest CLEAN remains to be rebuilt. Latest test Default build fails AL0122 at EAAgentDispatcherTest line 652 (Label to SecretText conversion). No runtime tests or publication: isolated test company is unavailable and administrative NAV dispatch is busy. This is a local, unverified-runtime checkpoint, not a completed fix.

Copilot-Session: 8ddc01d7-bd46-41c6-af55-61adbebea2b3

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

Copilot-Session: a1112d7a-1712-4a8f-ac8c-dc2c78b0ec9a
@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 Ownership: Needs Review Ownership is Other, low confidence, or needs manual correction Team: Other GitHub request for other area than SCM, Finance or Integration labels Sep 18, 2026
@github-actions github-actions Bot added this to the Version 30.0 milestone Sep 18, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9131e2a8-a4b5-40c8-a748-caade17b55c8

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove obsolete hidden mailbox assist-edit handlers from the legacy setup card while retaining the active wizard and table paths.

Copilot-Session: 9131e2a8-a4b5-40c8-a748-caade17b55c8

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep scheduler repair persistence under the existing update lock while making the mutation visible to CodeCop, and align lifecycle test fixtures with repository analyzer conventions.

Copilot-Session: 9131e2a8-a4b5-40c8-a748-caade17b55c8

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@t-prda Prangshuman Das (t-prda) added Team: Finance GitHub request for Finance area and removed Team: Other GitHub request for other area than SCM, Finance or Integration Ownership: Needs Review Ownership is Other, low confidence, or needs manual correction labels Sep 21, 2026
Allow the lifecycle suites to run in AL-Go's disposable Empty Company while retaining their local safety guard, and scope inherent permissions to the optional email-account deletion integration.

Copilot-Session: 9131e2a8-a4b5-40c8-a748-caade17b55c8

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove the synchronous email-account deletion subscriber and its dedicated tests. Missing registrations continue to be repaired by the existing wizard and scheduler reconciliation paths.

Copilot-Session: 9131e2a8-a4b5-40c8-a748-caade17b55c8

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Rename buffer repair and communication scheduling APIs, remove company-name coupling from lifecycle tests, and use codeunit isolation so the dispatcher and welcome suites are selected by standard CI passes.

Copilot-Session: 9131e2a8-a4b5-40c8-a748-caade17b55c8

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Configure the Expense Agent capability and required privacy approvals inside the codeunit-isolated dispatcher test fixture so standard CI can execute the newly discovered tests without tenant preconfiguration.

Copilot-Session: 9131e2a8-a4b5-40c8-a748-caade17b55c8

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Clean only native email records owned by prior dispatcher-test accounts and scope queue assertions to the current fixture account, so standard Codeunit isolation can coexist with unrelated outbox data.

Copilot-Session: 9131e2a8-a4b5-40c8-a748-caade17b55c8

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove the missing-setup and saved-Canary endpoint tests, delete their fixture-only counters, and rely on the normal TestPermissions Disabled runner context instead of redundant table grants.

Copilot-Session: 9131e2a8-a4b5-40c8-a748-caade17b55c8

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Resolve the setup-wizard conflict by preserving main's temporary-buffer naming cleanup and this branch's staged mailbox repair with a single persisted scheduler reconciliation.

Copilot-Session: 9131e2a8-a4b5-40c8-a748-caade17b55c8

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

Copy link
Copy Markdown
Contributor

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.


🔄 How to trigger a new status check:

  1. 📤 Push a new commit to the PR branch, or
  2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.

This branch was successfully deployed

1 active (outdated) deployment
triage — 950e4677 Deployed Sep 21, 2026 by t-prda via Classify team ownership #5276
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Team: Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant