diff --git a/dataset/codereview.jsonl b/dataset/codereview.jsonl index 04cd99d3d..f9c4c7831 100644 --- a/dataset/codereview.jsonl +++ b/dataset/codereview.jsonl @@ -108,7 +108,7 @@ {"repo": "microsoft/BCApps", "instance_id": "synthetic__caption-clean-01", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-17T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "style", "articles": ["ui/caption-capitalization-noun-phrase-vs-sentence-phrase"]}, "patch": "diff --git a/src/DemoCap/DemoCapDoc.Table.al b/src/DemoCap/DemoCapDoc.Table.al\nnew file mode 100644\nindex 0000000..3d12726\n--- /dev/null\n+++ b/src/DemoCap/DemoCapDoc.Table.al\n@@ -0,0 +1,27 @@\n+table 50190 \"Demo Cap Doc\"\n+{\n+ Caption = 'Demo Cap Doc';\n+ DataClassification = CustomerContent;\n+\n+ fields\n+ {\n+ field(1; \"No.\"; Code[20])\n+ {\n+ Caption = 'No.';\n+ DataClassification = CustomerContent;\n+ }\n+ field(2; Description; Text[100])\n+ {\n+ Caption = 'Description';\n+ DataClassification = CustomerContent;\n+ }\n+ }\n+\n+ keys\n+ {\n+ key(PK; \"No.\")\n+ {\n+ Clustered = true;\n+ }\n+ }\n+}\n\\ No newline at end of file\ndiff --git a/src/DemoCap/DemoCapList.Page.al b/src/DemoCap/DemoCapList.Page.al\nnew file mode 100644\nindex 0000000..353568d\n--- /dev/null\n+++ b/src/DemoCap/DemoCapList.Page.al\n@@ -0,0 +1,47 @@\n+page 50191 \"Demo Cap List\"\n+{\n+ PageType = List;\n+ ApplicationArea = All;\n+ UsageCategory = Lists;\n+ SourceTable = \"Demo Cap Doc\";\n+ Caption = 'Demo Cap List';\n+\n+ layout\n+ {\n+ area(Content)\n+ {\n+ repeater(Lines)\n+ {\n+ field(\"No.\"; Rec.\"No.\")\n+ {\n+ ToolTip = 'Specifies the number of the document.';\n+ }\n+ field(Description; Rec.Description)\n+ {\n+ ToolTip = 'Specifies a description of the document.';\n+ }\n+ }\n+ }\n+ }\n+\n+ actions\n+ {\n+ area(Processing)\n+ {\n+ action(ShowSourceDocument)\n+ {\n+ Caption = 'Show source document';\n+ Image = ViewSourceDocumentLine;\n+ ToolTip = 'Open the related source document.';\n+\n+ trigger OnAction()\n+ begin\n+ Message(ShowSourceMsg, Rec.\"No.\");\n+ end;\n+ }\n+ }\n+ }\n+\n+ var\n+ ShowSourceMsg: Label 'Showing source document %1.', Comment = '%1 = document number';\n+}\n\\ No newline at end of file", "expected_comments": [], "category": "code-review", "description": "Negative example: an action Caption uses sentence case ('Show source document'). Sentence-phrase action captions are correct in sentence case, so a title-case style finding is a false positive. Exercises false-positive rate. Derived from online-eval feedback (PR 8173).", "expect_findings": false, "source": "vsoadmin"} {"repo": "microsoft/BCApps", "instance_id": "synthetic__pkget-clean-01", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-06-16T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "performance", "articles": ["performance/primary-key-get-in-loop-is-transaction-cached"]}, "patch": "diff --git a/src/DemoBin/DemoBin.Table.al b/src/DemoBin/DemoBin.Table.al\nnew file mode 100644\nindex 0000000..d8fb076\n--- /dev/null\n+++ b/src/DemoBin/DemoBin.Table.al\n@@ -0,0 +1,32 @@\n+table 50192 \"Demo Bin\"\n+{\n+ Caption = 'Demo Bin';\n+ DataClassification = CustomerContent;\n+\n+ fields\n+ {\n+ field(1; \"Location Code\"; Code[10])\n+ {\n+ Caption = 'Location Code';\n+ DataClassification = CustomerContent;\n+ }\n+ field(2; \"Code\"; Code[20])\n+ {\n+ Caption = 'Code';\n+ DataClassification = CustomerContent;\n+ }\n+ field(3; \"Bin Type Code\"; Code[10])\n+ {\n+ Caption = 'Bin Type Code';\n+ DataClassification = CustomerContent;\n+ }\n+ }\n+\n+ keys\n+ {\n+ key(PK; \"Location Code\", \"Code\")\n+ {\n+ Clustered = true;\n+ }\n+ }\n+}\n\\ No newline at end of file\ndiff --git a/src/DemoBin/DemoBinCheck.Codeunit.al b/src/DemoBin/DemoBinCheck.Codeunit.al\nnew file mode 100644\nindex 0000000..cfef62c\n--- /dev/null\n+++ b/src/DemoBin/DemoBinCheck.Codeunit.al\n@@ -0,0 +1,20 @@\n+codeunit 50194 \"Demo Bin Check\"\n+{\n+ procedure ErrorIfReceiveBin(LocationCode: Code[10]; BinCode: Code[20])\n+ var\n+ DemoBin: Record \"Demo Bin\";\n+ DemoBinType: Record \"Demo Bin Type\";\n+ begin\n+ if (LocationCode = '') or (BinCode = '') then\n+ exit;\n+ if not DemoBin.Get(LocationCode, BinCode) then\n+ exit;\n+ if not DemoBinType.Get(DemoBin.\"Bin Type Code\") then\n+ exit;\n+ if DemoBinType.Receive then\n+ Error(CannotUseReceiveBinErr, BinCode);\n+ end;\n+\n+ var\n+ CannotUseReceiveBinErr: Label 'Bin %1 is a receive bin and cannot be used here.', Comment = '%1 = bin code';\n+}\n\\ No newline at end of file\ndiff --git a/src/DemoBin/DemoBinType.Table.al b/src/DemoBin/DemoBinType.Table.al\nnew file mode 100644\nindex 0000000..95a66bc\n--- /dev/null\n+++ b/src/DemoBin/DemoBinType.Table.al\n@@ -0,0 +1,27 @@\n+table 50193 \"Demo Bin Type\"\n+{\n+ Caption = 'Demo Bin Type';\n+ DataClassification = CustomerContent;\n+\n+ fields\n+ {\n+ field(1; \"Code\"; Code[10])\n+ {\n+ Caption = 'Code';\n+ DataClassification = CustomerContent;\n+ }\n+ field(2; Receive; Boolean)\n+ {\n+ Caption = 'Receive';\n+ DataClassification = CustomerContent;\n+ }\n+ }\n+\n+ keys\n+ {\n+ key(PK; \"Code\")\n+ {\n+ Clustered = true;\n+ }\n+ }\n+}\n\\ No newline at end of file", "expected_comments": [], "category": "code-review", "description": "Negative example: a per-row helper performs two guarded primary-key Get calls (Bin and Bin Type). Primary-key Gets are cached within the transaction and per-row keys differ, so an N+1 or dictionary-cache performance finding is a false positive. Exercises false-positive rate. Derived from online-eval feedback (PR 8584).", "expect_findings": false, "source": "vsoadmin"} {"base_commit": "0abbd4e2f907f66c999cd49f4f7cd42c114a8031", "category": "code-review", "created_at": "2026-08-04T00:00:00Z", "description": "False-positive guard: relocating a shipped field to a same-app tableextension with the same field ID and name (ES 'G/L Entry' field 7000000 'Bill No.' moved to tableextension 7000017 'G/L Entry ES') is a relocation, not a deletion or breaking change, and needs no ObsoleteState staging; the base-table key that referenced the field is necessarily trimmed. Expect no findings (BCApps PR 9607).", "environment_setup_version": "29.0", "expected_comments": [], "expect_findings": false, "instance_id": "synthetic__breaking-relocation-01", "metadata": {"area": "breaking-changes", "articles": ["breaking-changes/relocating-a-field-to-a-tableextension-is-not-a-deletion"]}, "patch": "diff --git a/src/Layers/ES/BaseApp/Finance/GeneralLedger/Ledger/GLEntry.Table.al b/src/Layers/ES/BaseApp/Finance/GeneralLedger/Ledger/GLEntry.Table.al\nindex 5de482ebc1..8a5f3953b3 100644\n--- a/src/Layers/ES/BaseApp/Finance/GeneralLedger/Ledger/GLEntry.Table.al\n+++ b/src/Layers/ES/BaseApp/Finance/GeneralLedger/Ledger/GLEntry.Table.al\n@@ -789,10 +789,6 @@ table 17 \"G/L Entry\"\n Caption = 'Period Trans. No.';\n DataClassification = SystemMetadata;\n }\n- field(7000000; \"Bill No.\"; Code[20])\n- {\n- Caption = 'Bill No.';\n- }\n }\n \n keys\n@@ -844,7 +840,7 @@ table 17 \"G/L Entry\"\n key(Key12; \"VAT Bus. Posting Group\", \"VAT Prod. Posting Group\")\n {\n }\n- key(Key13; \"G/L Account No.\", \"Document No.\", \"Bill No.\")\n+ key(Key13; \"G/L Account No.\", \"Document No.\")\n {\n SumIndexFields = Amount, \"Additional-Currency Amount\";\n }\ndiff --git a/src/Layers/ES/BaseApp/Local/Finance/GeneralLedger/Ledger/GLEntryES.TableExt.al b/src/Layers/ES/BaseApp/Local/Finance/GeneralLedger/Ledger/GLEntryES.TableExt.al\nnew file mode 100644\nindex 0000000000..d3cbda2a81\n--- /dev/null\n+++ b/src/Layers/ES/BaseApp/Local/Finance/GeneralLedger/Ledger/GLEntryES.TableExt.al\n@@ -0,0 +1,17 @@\n+// ------------------------------------------------------------------------------------------------\n+// Copyright (c) Microsoft Corporation. All rights reserved.\n+// Licensed under the MIT License. See License.txt in the project root for license information.\n+// ------------------------------------------------------------------------------------------------\n+namespace Microsoft.Finance.GeneralLedger.Ledger;\n+\n+tableextension 7000017 \"G/L Entry ES\" extends \"G/L Entry\"\n+{\n+ fields\n+ {\n+ field(7000000; \"Bill No.\"; Code[20])\n+ {\n+ Caption = 'Bill No.';\n+ DataClassification = CustomerContent;\n+ }\n+ }\n+}\n\\ No newline at end of file\n", "project_paths": [], "repo": "microsoft/BCApps", "source": "vsoadmin"} -{"repo":"microsoft/BCApps","instance_id":"synthetic__upgrade-obsolete-caption-01","base_commit":"36819ac5ecb7b3d39608acd649e2459d035c6985","created_at":"2026-08-14T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"upgrade"},"patch":"diff --git a/src/SampleSubcWorkCenterCard.Page.al b/src/SampleSubcWorkCenterCard.Page.al\nnew file mode 100644\nindex 0000000..e22443b\n--- /dev/null\n+++ b/src/SampleSubcWorkCenterCard.Page.al\n@@ -0,0 +1,32 @@\n+page 50140 \"Sample Work Center Card\"\n+{\n+ PageType = Card;\n+ ApplicationArea = All;\n+ UsageCategory = None;\n+ SourceTable = \"Work Center\";\n+\n+ layout\n+ {\n+ area(content)\n+ {\n+ field(\"No.\"; Rec.\"No.\")\n+ {\n+ ApplicationArea = All;\n+ }\n+ }\n+ }\n+\n+ actions\n+ {\n+ area(Reporting)\n+ {\n+ action(\"Sample Dispatch List (Obsolete)\")\n+ {\n+ ApplicationArea = All;\n+ Caption = 'Sample Dispatch List (Obsolete)';\n+ Image = Report;\n+ RunObject = report \"Sample Dispatch List\";\n+ }\n+ }\n+ }\n+}\n","expected_comments":[{"file":"src/SampleSubcWorkCenterCard.Page.al","line_start":23,"line_end":29,"severity":"high","domain":"upgrade","body":"Renaming this action's Caption to append '(Obsolete)' is not a substitute for the formal obsoletion lifecycle. The action is never entered into ObsoleteState/ObsoleteReason/ObsoleteTag, so it stays fully callable with no compiler warning and no structured migration guidance for consumers who reference it by name. Set ObsoleteState = Pending with an ObsoleteReason pointing to the replacement action/report and an ObsoleteTag recording the introducing version."}],"category":"code-review","description":"Genuine finding: a base page action is 'deprecated' by only appending \"(Obsolete)\" to its Caption while a replacement action is introduced elsewhere; no ObsoleteState/ObsoleteReason/ObsoleteTag is ever set, so the action keeps working with no compiler-enforced migration signal. Guards the caption-only-pseudo-deprecation boundary (BCApps PR 10080, WorkCenterCard.Page.al and its IT-layer duplicate).","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__upgrade-obsolete-caption-01","base_commit":"36819ac5ecb7b3d39608acd649e2459d035c6985","created_at":"2026-08-14T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"upgrade"},"patch":"diff --git a/src/SampleSubcWorkCenterCard.Page.al b/src/SampleSubcWorkCenterCard.Page.al\nnew file mode 100644\nindex 0000000..e22443b\n--- /dev/null\n+++ b/src/SampleSubcWorkCenterCard.Page.al\n@@ -0,0 +1,32 @@\n+page 50140 \"Sample Work Center Card\"\n+{\n+ PageType = Card;\n+ ApplicationArea = All;\n+ UsageCategory = None;\n+ SourceTable = \"Work Center\";\n+\n+ layout\n+ {\n+ area(content)\n+ {\n+ field(\"No.\"; Rec.\"No.\")\n+ {\n+ ApplicationArea = All;\n+ }\n+ }\n+ }\n+\n+ actions\n+ {\n+ area(Reporting)\n+ {\n+ action(\"Sample Dispatch List (Obsolete)\")\n+ {\n+ ApplicationArea = All;\n+ Caption = 'Sample Dispatch List (Obsolete)';\n+ Image = Report;\n+ RunObject = report \"Sample Dispatch List\";\n+ }\n+ }\n+ }\n+}\n","expected_comments":[{"file":"src/SampleSubcWorkCenterCard.Page.al","line_start":23,"line_end":29,"severity":"high","domain":"upgrade","body":"Renaming this action's Caption to append '(Obsolete)' is not a substitute for the formal obsoletion lifecycle. The action is never entered into ObsoleteState/ObsoleteReason/ObsoleteTag, so it stays fully callable with no compiler warning and no structured migration guidance for consumers who reference it by name. Set ObsoleteState = Pending with an ObsoleteReason pointing to the replacement action/report and an ObsoleteTag recording the introducing version.","articles":["upgrade/obsoletion-requires-reason-and-tag"]}],"category":"code-review","description":"Genuine finding: a base page action is 'deprecated' by only appending \"(Obsolete)\" to its Caption while a replacement action is introduced elsewhere; no ObsoleteState/ObsoleteReason/ObsoleteTag is ever set, so the action keeps working with no compiler-enforced migration signal. Guards the caption-only-pseudo-deprecation boundary (BCApps PR 10080, WorkCenterCard.Page.al and its IT-layer duplicate).","expect_findings":true,"source":"vsoadmin"} {"repo":"microsoft/BCApps","instance_id":"synthetic__style-duplicate-action-01","base_commit":"36819ac5ecb7b3d39608acd649e2459d035c6985","created_at":"2026-08-14T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"style"},"patch":"diff --git a/src/SampleWorkCenterCardBase.Page.al b/src/SampleWorkCenterCardBase.Page.al\nnew file mode 100644\nindex 0000000..a4a85b0\n--- /dev/null\n+++ b/src/SampleWorkCenterCardBase.Page.al\n@@ -0,0 +1,35 @@\n+page 50141 \"Sample Work Center Card Base\"\n+{\n+ PageType = Card;\n+ ApplicationArea = All;\n+ UsageCategory = None;\n+ SourceTable = \"Work Center\";\n+\n+ layout\n+ {\n+ area(content)\n+ {\n+ field(\"No.\"; Rec.\"No.\")\n+ {\n+ ApplicationArea = All;\n+ }\n+ }\n+ }\n+\n+ actions\n+ {\n+ area(Reporting)\n+ {\n+ action(\"Sample Dispatch List\")\n+ {\n+ ApplicationArea = All;\n+ Caption = 'Sample Dispatch List (Obsolete)';\n+ Image = Report;\n+ RunObject = report \"Sample Dispatch List\";\n+ ObsoleteState = Pending;\n+ ObsoleteReason = 'Replaced by action \"Sample Dispatch List\" on the Sample Work Center Card Ext page extension, which runs the report filtered to the work center''s vendor.';\n+ ObsoleteTag = '29.0';\n+ }\n+ }\n+ }\n+}\ndiff --git a/src/SampleWorkCenterCardExt.PageExt.al b/src/SampleWorkCenterCardExt.PageExt.al\nnew file mode 100644\nindex 0000000..4c93a6b\n--- /dev/null\n+++ b/src/SampleWorkCenterCardExt.PageExt.al\n@@ -0,0 +1,21 @@\n+pageextension 50142 \"Sample Work Center Card Ext\" extends \"Sample Work Center Card Base\"\n+{\n+ actions\n+ {\n+ addlast(processing)\n+ {\n+ group(\"Sample Group\")\n+ {\n+ Caption = 'Sample Group';\n+\n+ action(\"Sample Dispatch List New\")\n+ {\n+ ApplicationArea = All;\n+ Caption = 'Sample Dispatch List';\n+ Image = Report;\n+ RunObject = report \"Sample Dispatch List\";\n+ }\n+ }\n+ }\n+ }\n+}\n","expected_comments":[],"category":"code-review","description":"False-positive guard: a page extension adds a new report action under its own custom action group instead of reusing the base page's area(Reporting) placement, while the base page's existing report action of the same purpose is already formally obsoleted (ObsoleteState/Reason/Tag set) and its Caption differentiated with an '(Obsolete)' suffix. This is a normal deprecate-and-replace transition, not a confusing duplicate-caption/ribbon-placement problem, so no finding is expected (BCApps PR 10080).","expect_findings":false,"source":"vsoadmin"} {"repo":"microsoft/BCApps","instance_id":"synthetic__error-testfield-enabled-01","base_commit":"36819ac5ecb7b3d39608acd649e2459d035c6985","created_at":"2026-08-14T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"error-handling"},"patch":"diff --git a/src/SampleWorkCenter.Table.al b/src/SampleWorkCenter.Table.al\nnew file mode 100644\nindex 0000000..0bfc8d5\n--- /dev/null\n+++ b/src/SampleWorkCenter.Table.al\n@@ -0,0 +1,24 @@\n+table 50150 \"Sample Work Center\"\n+{\n+ DataClassification = CustomerContent;\n+\n+ fields\n+ {\n+ field(1; \"No.\"; Code[20])\n+ {\n+ DataClassification = CustomerContent;\n+ }\n+ field(2; \"Sample Vendor No.\"; Code[20])\n+ {\n+ DataClassification = CustomerContent;\n+ }\n+ }\n+\n+ keys\n+ {\n+ key(PK; \"No.\")\n+ {\n+ Clustered = true;\n+ }\n+ }\n+}\ndiff --git a/src/SampleWorkCenterCard.Page.al b/src/SampleWorkCenterCard.Page.al\nnew file mode 100644\nindex 0000000..b83bc7f\n--- /dev/null\n+++ b/src/SampleWorkCenterCard.Page.al\n@@ -0,0 +1,22 @@\n+page 50151 \"Sample Work Center Card\"\n+{\n+ PageType = Card;\n+ ApplicationArea = All;\n+ UsageCategory = None;\n+ SourceTable = \"Sample Work Center\";\n+\n+ layout\n+ {\n+ area(content)\n+ {\n+ field(\"No.\"; Rec.\"No.\")\n+ {\n+ ApplicationArea = All;\n+ }\n+ field(\"Sample Vendor No.\"; Rec.\"Sample Vendor No.\")\n+ {\n+ ApplicationArea = All;\n+ }\n+ }\n+ }\n+}\ndiff --git a/src/SampleWorkCenterCardExt.PageExt.al b/src/SampleWorkCenterCardExt.PageExt.al\nnew file mode 100644\nindex 0000000..c3bd48f\n--- /dev/null\n+++ b/src/SampleWorkCenterCardExt.PageExt.al\n@@ -0,0 +1,24 @@\n+pageextension 50152 \"Sample Work Center Card Ext\" extends \"Sample Work Center Card\"\n+{\n+ actions\n+ {\n+ addlast(processing)\n+ {\n+ action(\"Sample Dispatch List\")\n+ {\n+ ApplicationArea = All;\n+ Caption = 'Sample Dispatch List';\n+ Image = Report;\n+ Enabled = Rec.\"Sample Vendor No.\" <> '';\n+\n+ trigger OnAction()\n+ var\n+ PurchaseHeader: Record \"Purchase Header\";\n+ begin\n+ PurchaseHeader.SetRange(\"Buy-from Vendor No.\", Rec.\"Sample Vendor No.\");\n+ Report.Run(Report::\"Sample Dispatch List\", true, false, PurchaseHeader);\n+ end;\n+ }\n+ }\n+ }\n+}\n","expected_comments":[],"category":"code-review","description":"False-positive guard: an action's OnAction trigger filters and runs a report using a field without a TestField call, but the action's Enabled property is bound to that same field being non-blank, so the trigger cannot be invoked through the UI while the field is empty. A finding demanding a redundant TestField here is a false positive (BCApps PR 10080, SubcWorkCenterCard.PageExt.al).","expect_findings":false,"source":"vsoadmin"} {"repo": "microsoft/BCApps", "instance_id": "synthetic__privacy-sibling-override-01", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-08-05T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "privacy"}, "patch": "diff --git a/src/DemoSpendLink/DemoSpendLink.Table.al b/src/DemoSpendLink/DemoSpendLink.Table.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoSpendLink/DemoSpendLink.Table.al\n@@ -0,0 +1,30 @@\n+table 50130 \"Demo Spend Link\"\n+{\n+ Caption = 'Demo Spend Link';\n+ DataClassification = SystemMetadata;\n+\n+ fields\n+ {\n+ field(1; \"Entry No.\"; Integer)\n+ {\n+ Caption = 'Entry No.';\n+ }\n+ field(2; \"Request No.\"; Code[20])\n+ {\n+ Caption = 'Request No.';\n+ DataClassification = CustomerContent;\n+ }\n+ field(3; \"Amount\"; Decimal)\n+ {\n+ Caption = 'Amount';\n+ }\n+ }\n+\n+ keys\n+ {\n+ key(PK; \"Entry No.\")\n+ {\n+ Clustered = true;\n+ }\n+ }\n+}\n", "expected_comments": [{"file": "src/DemoSpendLink/DemoSpendLink.Table.al", "line_start": 17, "line_end": 17, "severity": "medium", "domain": "privacy", "body": "Table declares DataClassification = SystemMetadata, but field(3) \"Amount\" holds transactional data, not system metadata. \"Request No.\" on the same table already overrides to CustomerContent, so \"Amount\" should get the same override.", "articles": ["privacy/table-level-data-classification-cascades"]}], "category": "code-review", "description": "Accepted feedback: a table default of DataClassification = SystemMetadata does not cover a transactional field left un-overridden when a sibling field on the same table already overrides to CustomerContent; the reviewer must flag the remaining field(s) for the same override (BCApps PR 8959, SpendRequestToGLLink.Table.al, Amount/Posting Date/G/L Account No. left as SystemMetadata while Spend Request No. was already overridden).", "expect_findings": true, "source": "vsoadmin"} @@ -118,7 +118,7 @@ {"repo":"microsoft/BCApps","instance_id":"synthetic__style-this-keyword-01","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-08-05T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"style"},"patch":"diff --git a/src/DemoThisKeyword/DemoTestHelpers.Codeunit.al b/src/DemoThisKeyword/DemoTestHelpers.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoThisKeyword/DemoTestHelpers.Codeunit.al\n@@ -0,0 +1,22 @@\n+codeunit 50141 \"Demo Test Helpers\"\n+{\n+ Subtype = Test;\n+\n+ [Test]\n+ procedure TestCreateRequest()\n+ var\n+ No: Code[20];\n+ begin\n+ Initialize();\n+ No := CreateRequest();\n+ end;\n+\n+ local procedure Initialize()\n+ begin\n+ end;\n+\n+ local procedure CreateRequest(): Code[20]\n+ begin\n+ exit('DEMO001');\n+ end;\n+}\n","expected_comments":[],"category":"code-review","description":"False-positive guard: bare calls to a codeunit's own local procedures (Initialize(), CreateRequest()) do not need a 'this.' qualifier; BCApps style rejects the 'this.' prefix for self-references in codeunits, so an AA0248-style finding demanding 'this.' on every self-call is a false positive (BCApps PR 8959, SpendRequestTests.Codeunit.al; developer reply: \"Wrong. We should not use 'this'\").","expect_findings":false,"source":"vsoadmin"} {"repo": "microsoft/BCApps", "instance_id": "synthetic__errh-tryfunction-swallowed-01", "base_commit": "72d9d6dd801f26fa953fa5258e42774808857b03", "created_at": "2026-08-05T00:00:00Z", "environment_setup_version": "29.0", "project_paths": [], "metadata": {"area": "error-handling"}, "patch": "diff --git a/src/DemoSelector/DemoSelectorClient.Codeunit.al b/src/DemoSelector/DemoSelectorClient.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoSelector/DemoSelectorClient.Codeunit.al\n@@ -0,0 +1,25 @@\n+codeunit 50182 \"Demo Selector Client\"\n+{\n+ Access = Internal;\n+\n+ internal procedure ResolveBestMatch(Payload: Text): Text[100]\n+ var\n+ BestMatch: Text[100];\n+ begin\n+ if not TryFindBestMatch(Payload, BestMatch) then\n+ exit('');\n+\n+ exit(BestMatch);\n+ end;\n+\n+ [TryFunction]\n+ local procedure TryFindBestMatch(Payload: Text; var BestMatch: Text[100])\n+ var\n+ JsonResponse: JsonObject;\n+ MatchToken: JsonToken;\n+ begin\n+ JsonResponse.ReadFrom(Payload);\n+ JsonResponse.Get('bestMatch', MatchToken);\n+ BestMatch := CopyStr(MatchToken.AsValue().AsText(), 1, MaxStrLen(BestMatch));\n+ end;\n+}\ndiff --git a/src/DemoSelector/app.json b/src/DemoSelector/app.json\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoSelector/app.json\n@@ -0,0 +1,17 @@\n+{\n+ \"id\": \"f1a2b3c4-5d6e-4708-9182-3a4b5c6d7e80\",\n+ \"name\": \"Demo Selector\",\n+ \"publisher\": \"Contoso\",\n+ \"version\": \"1.0.0.0\",\n+ \"brief\": \"Demo AI selector client.\",\n+ \"description\": \"Demonstrates a TryFunction result consumed without diagnostics on failure.\",\n+ \"platform\": \"1.0.0.0\",\n+ \"application\": \"29.0.0.0\",\n+ \"idRanges\": [\n+ {\n+ \"from\": 50180,\n+ \"to\": 50189\n+ }\n+ ],\n+ \"runtime\": \"15.0\"\n+}\n", "expected_comments": [{"file": "src/DemoSelector/DemoSelectorClient.Codeunit.al", "line_start": 9, "line_end": 9, "severity": "medium", "domain": "error-handling", "body": "The [TryFunction] result of TryFindBestMatch is consumed, but the false branch is returned immediately with no diagnostics. If the JSON payload is malformed or the expected key is missing, the caller only sees a blank result and treats it like an empty selection, discarding the last-error context instead of logging or surfacing the failure while it is still available (BCApps PR 9364, discussion r3677232004).", "articles": ["performance/use-tryfunction-for-error-catching-not-rollback"]}], "category": "code-review", "description": "True positive: a [TryFunction] return value is checked but the false branch swallows the failure without reading GetLastErrorText/GetLastErrorCallStack, so a technical parsing failure is indistinguishable from a valid empty selection. Mirrors BCApps PR 9364's SOAItemSelector.Codeunit.al TrySelectBestMatchingItem finding (accepted, discussion r3677232004): ignoring a TryFunction's error context disables its diagnostic value even though the boolean short-circuit itself is legitimate.", "expect_findings": true, "source": "vsoadmin"} {"repo": "microsoft/BCApps", "instance_id": "synthetic__errh-errortype-internal-01", "base_commit": "72d9d6dd801f26fa953fa5258e42774808857b03", "created_at": "2026-08-05T00:00:00Z", "environment_setup_version": "29.0", "project_paths": [], "metadata": {"area": "error-handling"}, "patch": "diff --git a/src/DemoSessionGuard/DemoSessionContentReader.Codeunit.al b/src/DemoSessionGuard/DemoSessionContentReader.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoSessionGuard/DemoSessionContentReader.Codeunit.al\n@@ -0,0 +1,20 @@\n+codeunit 50190 \"Demo Session Content Reader\"\n+{\n+ Access = Internal;\n+\n+ internal procedure GetContent(ExpectedContextId: Guid; ActualContextId: Guid): Text\n+ begin\n+ if ActualContextId <> ExpectedContextId then\n+ Error(ContextMismatchErr);\n+\n+ exit(GetContentInternal());\n+ end;\n+\n+ local procedure GetContentInternal(): Text\n+ begin\n+ exit('content');\n+ end;\n+\n+ var\n+ ContextMismatchErr: Label 'The requested context does not match the current session context.';\n+}\ndiff --git a/src/DemoSessionGuard/app.json b/src/DemoSessionGuard/app.json\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoSessionGuard/app.json\n@@ -0,0 +1,17 @@\n+{\n+ \"id\": \"a2b3c4d5-6e7f-4819-a293-4b5c6d7e8f91\",\n+ \"name\": \"Demo Session Guard\",\n+ \"publisher\": \"Contoso\",\n+ \"version\": \"1.0.0.0\",\n+ \"brief\": \"Demo internal session guard.\",\n+ \"description\": \"Demonstrates an internal invariant guard raised as a plain Error instead of ErrorInfo ErrorType::Internal.\",\n+ \"platform\": \"1.0.0.0\",\n+ \"application\": \"29.0.0.0\",\n+ \"idRanges\": [\n+ {\n+ \"from\": 50190,\n+ \"to\": 50199\n+ }\n+ ],\n+ \"runtime\": \"15.0\"\n+}\n", "expected_comments": [{"file": "src/DemoSessionGuard/DemoSessionContentReader.Codeunit.al", "line_start": 8, "line_end": 8, "severity": "high", "domain": "error-handling", "body": "This session/context-mismatch check is an internal invariant guard, not user-actionable validation, but it raises a plain Error() so it can surface a technical 'context does not match' message to end users instead of being reported as an internal diagnostic. Raise an ErrorInfo with ErrorType::Internal for this guard (BCApps PR 9364, discussion r3681389575).", "articles": ["error-handling/errortype-internal-vs-client-for-diagnostics"]}], "category": "code-review", "description": "True positive: an internal session-guard check (caller/context mismatch) raises a plain Error() intended for end users, when it is really an internal invariant. Mirrors BCApps PR 9364's SOATaskMessageReader.Codeunit.al finding (accepted, discussion r3681389575): internal guard failures should use ErrorInfo ErrorType::Internal rather than a plain end-user Error.", "expect_findings": true, "source": "vsoadmin"} -{"repo":"microsoft/BCApps","instance_id":"synthetic__sec-internal-not-boundary-01","base_commit":"72d9d6dd801f26fa953fa5258e42774808857b03","created_at":"2026-08-05T00:00:00Z","environment_setup_version":"29.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/DemoTaskReader/DemoTaskMessage.Table.al b/src/DemoTaskReader/DemoTaskMessage.Table.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoTaskReader/DemoTaskMessage.Table.al\n@@ -0,0 +1,30 @@\n+table 50200 \"Demo Task Message\"\n+{\n+ Caption = 'Demo Task Message';\n+ DataClassification = CustomerContent;\n+\n+ fields\n+ {\n+ field(1; \"Entry No.\"; Integer)\n+ {\n+ Caption = 'Entry No.';\n+ AutoIncrement = true;\n+ }\n+ field(2; \"Task Id\"; BigInteger)\n+ {\n+ Caption = 'Task Id';\n+ }\n+ field(3; Content; Text[2048])\n+ {\n+ Caption = 'Content';\n+ }\n+ }\n+\n+ keys\n+ {\n+ key(PK; \"Entry No.\")\n+ {\n+ Clustered = true;\n+ }\n+ }\n+}\ndiff --git a/src/DemoTaskReader/DemoTaskMessageReader.Codeunit.al b/src/DemoTaskReader/DemoTaskMessageReader.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoTaskReader/DemoTaskMessageReader.Codeunit.al\n@@ -0,0 +1,16 @@\n+codeunit 50201 \"Demo Task Message Reader\"\n+{\n+ Access = Internal;\n+ Permissions = tabledata \"Demo Task Message\" = r;\n+\n+ internal procedure GetLatestMessageContent(TaskId: BigInteger): Text\n+ var\n+ TaskMessage: Record \"Demo Task Message\";\n+ begin\n+ TaskMessage.SetRange(\"Task Id\", TaskId);\n+ if TaskMessage.FindLast() then\n+ exit(TaskMessage.Content);\n+\n+ exit('');\n+ end;\n+}\ndiff --git a/src/DemoTaskReader/app.json b/src/DemoTaskReader/app.json\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoTaskReader/app.json\n@@ -0,0 +1,17 @@\n+{\n+ \"id\": \"b3c4d5e6-7f80-492a-b3a4-5c6d7e8f9012\",\n+ \"name\": \"Demo Task Reader\",\n+ \"publisher\": \"Contoso\",\n+ \"version\": \"1.0.0.0\",\n+ \"brief\": \"Demo privileged task message reader.\",\n+ \"description\": \"Demonstrates an internal helper with elevated table permissions that does not re-verify the caller's own session/task context.\",\n+ \"platform\": \"1.0.0.0\",\n+ \"application\": \"29.0.0.0\",\n+ \"idRanges\": [\n+ {\n+ \"from\": 50200,\n+ \"to\": 50209\n+ }\n+ ],\n+ \"runtime\": \"15.0\"\n+}\n","expected_comments":[{"file":"src/DemoTaskReader/DemoTaskMessageReader.Codeunit.al","line_start":6,"line_end":6,"severity":"medium","domain":"security","body":"GetLatestMessageContent is exposed as an internal helper with elevated `Permissions = tabledata \"Demo Task Message\" = r`, but it never verifies that the requested TaskId belongs to the current caller's own session/task context before returning that task's content. `internal` narrows the set of supported callers but is not itself an authorization boundary, so any other internal caller could reuse this helper as a confused deputy to read another task's message content. Assert the requested TaskId matches the caller's own context inside the helper rather than relying on every current and future internal caller to only pass its own id (BCApps PR 9364, discussion r3678258180)."}],"category":"code-review","description":"True positive: an internal helper with privileged tabledata read permissions reads and returns another record's content keyed only by a caller-supplied id, without asserting that id belongs to the caller's own session/task. Mirrors BCApps PR 9364's SOATaskMessageReader.Codeunit.al finding (accepted, discussion r3678258180): internal access modifiers are not a security boundary, so this is a defense-in-depth confused-deputy risk that must be flagged even though it is not directly exploitable by today's single call site.","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__sec-internal-not-boundary-01","base_commit":"72d9d6dd801f26fa953fa5258e42774808857b03","created_at":"2026-08-05T00:00:00Z","environment_setup_version":"29.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/DemoTaskReader/DemoTaskMessage.Table.al b/src/DemoTaskReader/DemoTaskMessage.Table.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoTaskReader/DemoTaskMessage.Table.al\n@@ -0,0 +1,30 @@\n+table 50200 \"Demo Task Message\"\n+{\n+ Caption = 'Demo Task Message';\n+ DataClassification = CustomerContent;\n+\n+ fields\n+ {\n+ field(1; \"Entry No.\"; Integer)\n+ {\n+ Caption = 'Entry No.';\n+ AutoIncrement = true;\n+ }\n+ field(2; \"Task Id\"; BigInteger)\n+ {\n+ Caption = 'Task Id';\n+ }\n+ field(3; Content; Text[2048])\n+ {\n+ Caption = 'Content';\n+ }\n+ }\n+\n+ keys\n+ {\n+ key(PK; \"Entry No.\")\n+ {\n+ Clustered = true;\n+ }\n+ }\n+}\ndiff --git a/src/DemoTaskReader/DemoTaskMessageReader.Codeunit.al b/src/DemoTaskReader/DemoTaskMessageReader.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoTaskReader/DemoTaskMessageReader.Codeunit.al\n@@ -0,0 +1,16 @@\n+codeunit 50201 \"Demo Task Message Reader\"\n+{\n+ Access = Internal;\n+ Permissions = tabledata \"Demo Task Message\" = r;\n+\n+ internal procedure GetLatestMessageContent(TaskId: BigInteger): Text\n+ var\n+ TaskMessage: Record \"Demo Task Message\";\n+ begin\n+ TaskMessage.SetRange(\"Task Id\", TaskId);\n+ if TaskMessage.FindLast() then\n+ exit(TaskMessage.Content);\n+\n+ exit('');\n+ end;\n+}\ndiff --git a/src/DemoTaskReader/app.json b/src/DemoTaskReader/app.json\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoTaskReader/app.json\n@@ -0,0 +1,17 @@\n+{\n+ \"id\": \"b3c4d5e6-7f80-492a-b3a4-5c6d7e8f9012\",\n+ \"name\": \"Demo Task Reader\",\n+ \"publisher\": \"Contoso\",\n+ \"version\": \"1.0.0.0\",\n+ \"brief\": \"Demo privileged task message reader.\",\n+ \"description\": \"Demonstrates an internal helper with elevated table permissions that does not re-verify the caller's own session/task context.\",\n+ \"platform\": \"1.0.0.0\",\n+ \"application\": \"29.0.0.0\",\n+ \"idRanges\": [\n+ {\n+ \"from\": 50200,\n+ \"to\": 50209\n+ }\n+ ],\n+ \"runtime\": \"15.0\"\n+}\n","expected_comments":[{"file":"src/DemoTaskReader/DemoTaskMessageReader.Codeunit.al","line_start":6,"line_end":6,"severity":"medium","domain":"security","body":"GetLatestMessageContent is exposed as an internal helper with elevated `Permissions = tabledata \"Demo Task Message\" = r`, but it never verifies that the requested TaskId belongs to the current caller's own session/task context before returning that task's content. `internal` narrows the set of supported callers but is not itself an authorization boundary, so any other internal caller could reuse this helper as a confused deputy to read another task's message content. Assert the requested TaskId matches the caller's own context inside the helper rather than relying on every current and future internal caller to only pass its own id (BCApps PR 9364, discussion r3678258180).","articles":["security/internal-access-is-not-a-security-boundary"]}],"category":"code-review","description":"True positive: an internal helper with privileged tabledata read permissions reads and returns another record's content keyed only by a caller-supplied id, without asserting that id belongs to the caller's own session/task. Mirrors BCApps PR 9364's SOATaskMessageReader.Codeunit.al finding (accepted, discussion r3678258180): internal access modifiers are not a security boundary, so this is a defense-in-depth confused-deputy risk that must be flagged even though it is not directly exploitable by today's single call site.","expect_findings":true,"source":"vsoadmin"} {"repo": "microsoft/BCApps", "instance_id": "synthetic__breaking-access-modifier-01", "base_commit": "72d9d6dd801f26fa953fa5258e42774808857b03", "created_at": "2026-08-05T00:00:00Z", "environment_setup_version": "29.0", "project_paths": [], "metadata": {"area": "breaking-changes"}, "patch": "diff --git a/src/DemoVariantNormalizer/DemoVariantNormalizer.Codeunit.al b/src/DemoVariantNormalizer/DemoVariantNormalizer.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoVariantNormalizer/DemoVariantNormalizer.Codeunit.al\n@@ -0,0 +1,19 @@\n+codeunit 50210 \"Demo Variant Normalizer\"\n+{\n+ procedure NormalizeAlternatives(var AlternativeList: List of [Code[10]])\n+ begin\n+ TrimAlternatives(AlternativeList);\n+ end;\n+\n+ internal procedure TrimAlternatives(var AlternativeList: List of [Code[10]])\n+ var\n+ VariantCode: Code[10];\n+ Result: List of [Code[10]];\n+ begin\n+ foreach VariantCode in AlternativeList do\n+ if VariantCode <> '' then\n+ Result.Add(VariantCode);\n+\n+ AlternativeList := Result;\n+ end;\n+}\ndiff --git a/src/DemoVariantNormalizer/app.json b/src/DemoVariantNormalizer/app.json\nnew file mode 100644\n--- /dev/null\n+++ b/src/DemoVariantNormalizer/app.json\n@@ -0,0 +1,17 @@\n+{\n+ \"id\": \"c4d5e6f7-8091-4a3b-c4d5-6e7f80912345\",\n+ \"name\": \"Demo Variant Normalizer\",\n+ \"publisher\": \"Contoso\",\n+ \"version\": \"1.0.0.0\",\n+ \"brief\": \"Demo variant list normalizer.\",\n+ \"description\": \"Demonstrates an internal procedure that is only ever called from within the same object.\",\n+ \"platform\": \"1.0.0.0\",\n+ \"application\": \"29.0.0.0\",\n+ \"idRanges\": [\n+ {\n+ \"from\": 50210,\n+ \"to\": 50219\n+ }\n+ ],\n+ \"runtime\": \"15.0\"\n+}\n", "expected_comments": [{"file": "src/DemoVariantNormalizer/DemoVariantNormalizer.Codeunit.al", "line_start": 8, "line_end": 8, "severity": "medium", "domain": "breaking-changes", "body": "TrimAlternatives is declared internal even though it is only called from within the same object (NormalizeAlternatives). That widens the app's supported/exported surface unnecessarily; keep single-object helpers local unless another object genuinely needs to reuse them (BCApps PR 9364, discussion r3676578767).", "articles": ["breaking-changes/choose-access-modifiers-deliberately"]}], "category": "code-review", "description": "True positive: a helper procedure is declared internal but is only ever invoked from another procedure in the same codeunit. Mirrors BCApps PR 9364's SOAItemSearch.Codeunit.al NormalizeVariantAlternatives finding (accepted, discussion r3676578767): choose access modifiers deliberately -- an internal declaration for a same-object-only helper needlessly expands the app's exported surface and should be local.", "expect_findings": true, "source": "vsoadmin"} {"repo":"microsoft/BCApps","instance_id":"synthetic__testing-tolerance-clean-01","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-08-06T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"testing"},"patch":"diff --git a/src/DemoCFDI/DemoCFDIDateAssert.Codeunit.al b/src/DemoCFDI/DemoCFDIDateAssert.Codeunit.al\nnew file mode 100644\nindex 0000000..a1b2c3d\n--- /dev/null\n+++ b/src/DemoCFDI/DemoCFDIDateAssert.Codeunit.al\n@@ -0,0 +1,42 @@\n+codeunit 50201 \"Demo CFDI Date Assert\"\n+{\n+ Subtype = Test;\n+\n+ var\n+ Assert: Codeunit Assert;\n+ DateAssertionLbl: Label '%1. Expected: %2, Actual: %3 (difference: %4 days, tolerance: +/-1 day for timezone shifts)', Comment = '%1 = Error message, %2 = Expected date, %3 = Actual date, %4 = Days difference', Locked = true;\n+\n+ [Test]\n+ procedure StampDateAndPaymentDateAllowTimezoneTolerance()\n+ var\n+ StampDate: Date;\n+ PastWorkDate: Date;\n+ FechaValue: Text;\n+ FechaPagoValue: Text;\n+ begin\n+ StampDate := Today();\n+ PastWorkDate := CalcDate('<-30D>', StampDate);\n+\n+ FechaValue := FormatDateText(StampDate);\n+ FechaPagoValue := FormatDateText(PastWorkDate);\n+\n+ AssertDateWithinOneDayTolerance(StampDate, FechaValue, 'Comprobante/@Fecha date portion must be close to the stamp request date');\n+ AssertDateWithinOneDayTolerance(PastWorkDate, FechaPagoValue, 'FechaPago date portion must be close to the payment posting date');\n+\n+ Assert.AreNotEqual(CopyStr(FechaValue, 1, 10), CopyStr(FechaPagoValue, 1, 10), 'Comprobante/@Fecha and FechaPago must still differ');\n+ end;\n+\n+ local procedure FormatDateText(InputDate: Date): Text\n+ begin\n+ exit(Format(InputDate, 0, '--'));\n+ end;\n+\n+ local procedure AssertDateWithinOneDayTolerance(ExpectedDate: Date; ActualDateText: Text; ErrorMessage: Text)\n+ var\n+ DaysDiff: Integer;\n+ ActualDate: Date;\n+ begin\n+ Evaluate(ActualDate, CopyStr(ActualDateText, 1, 10), 9);\n+ DaysDiff := ActualDate - ExpectedDate;\n+ Assert.IsTrue(Abs(DaysDiff) <= 1, StrSubstNo(DateAssertionLbl, ErrorMessage, Format(ExpectedDate), CopyStr(ActualDateText, 1, 10), DaysDiff));\n+ end;\n+}\n","expected_comments":[],"category":"code-review","description":"Negative example: each serialized XML date is checked against its own expected date with a documented +/-1 day tolerance for timezone conversion. A separate assertion only verifies that the stamp and payment dates differ; it does not assert an exact 30-day gap. Flagging the bounded tolerance alone as masking an off-by-one bug is a false positive without concrete evidence that the accepted timezone variance is broader than intended (BCApps PR 9697, discussion r3701136019).","expect_findings":false,"source":"vsoadmin"} {"repo": "microsoft/BCApps", "instance_id": "synthetic__breaking-notification-callback-01", "base_commit": "b8b74184bba49dec7b0346b693331e32dd31508f", "created_at": "2026-08-14T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "breaking-changes"}, "patch": "diff --git a/src/SyntheticNotifMgmt.Codeunit.al b/src/SyntheticNotifMgmt.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SyntheticNotifMgmt.Codeunit.al\n@@ -0,0 +1,20 @@\n+codeunit 50130 \"Synthetic Notif Mgmt\"\n+{\n+ internal procedure ShowMissingLocationNotification(VendorNo: Code[20])\n+ var\n+ MissingLocationNotification: Notification;\n+ begin\n+ MissingLocationNotification.Message := 'The vendor does not have a location code assigned.';\n+ MissingLocationNotification.SetData('VendorNo', VendorNo);\n+ MissingLocationNotification.AddAction('Open vendor card', Codeunit::\"Synthetic Notif Mgmt\", 'OpenVendorCard');\n+ MissingLocationNotification.Send();\n+ end;\n+\n+ procedure OpenVendorCard(MissingLocationNotification: Notification)\n+ var\n+ Vendor: Record Vendor;\n+ begin\n+ if Vendor.Get(MissingLocationNotification.GetData('VendorNo')) then\n+ Page.Run(Page::\"Vendor Card\", Vendor);\n+ end;\n+}\n", "expected_comments": [{"file": "src/SyntheticNotifMgmt.Codeunit.al", "line_start": 13, "line_end": 13, "severity": "medium", "domain": "breaking-changes", "body": "OpenVendorCard is declared as a public procedure even though it is only used in-app as a Notification action callback, which unnecessarily publishes a new API surface that dependent extensions could bind to. Make it internal unless it is meant to be a stable external contract.", "articles": ["breaking-changes/choose-access-modifiers-deliberately"]}], "category": "code-review", "description": "Accepted feedback: a Notification action callback (OpenVendorCard) is declared as a bare public procedure even though it is only ever invoked in-app via Notification.AddAction, unnecessarily publishing a new API surface that dependent extensions could bind to. Per choose-access-modifiers-deliberately, notification callbacks with no external caller should default to internal. Expect a flagged finding on the callback procedure (BCApps PR 9787).", "expect_findings": true, "source": "vsoadmin"}