Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// ------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.
// ------------------------------------------------------------------------------------------------

namespace Microsoft.ExpenseAgent;

using System.Agents;

codeunit 7104 "EA Agent Archiving" implements IAgentArchiving
{
Access = Internal;
InherentEntitlements = X;
InherentPermissions = X;

procedure IsArchivingSupported(): Boolean
begin
exit(false);
end;
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ enumextension 6998 "EA Agent Metadata" extends "Agent Metadata Provider"
value(6998; "Expense Agent")
{
Caption = 'Expense Agent', Locked = true;
Implementation = IAgentFactory = "EA Metadata Provider", IAgentMetadata = "EA Metadata Provider", IAgentTaskExecution = "EA Agent Task Execution";
Implementation = IAgentFactory = "EA Metadata Provider", IAgentMetadata = "EA Metadata Provider", IAgentTaskExecution = "EA Agent Task Execution", IAgentArchiving = "EA Agent Archiving";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// ------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.
// ------------------------------------------------------------------------------------------------

namespace Microsoft.Agent.PayablesAgent;

using System.Agents;

codeunit 3319 "PA Agent Archiving" implements IAgentArchiving
{
Access = Internal;
InherentEntitlements = X;
InherentPermissions = X;

procedure IsArchivingSupported(): Boolean
begin
exit(false);
end;
}
2 changes: 1 addition & 1 deletion src/Apps/W1/PayablesAgent/app/PAAgentMetadata.EnumExt.al
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ enumextension 3304 "PA Agent Metadata" extends "Agent Metadata Provider"
value(3303; "Payables Agent")
{
Caption = 'Payables Agent', Locked = true;
Implementation = IAgentFactory = "Payables Agent", IAgentMetadata = "Payables Agent", IAgentTaskExecution = "PA Agent Task Execution";
Implementation = IAgentFactory = "Payables Agent", IAgentMetadata = "Payables Agent", IAgentTaskExecution = "PA Agent Task Execution", IAgentArchiving = "PA Agent Archiving";
}
}
14 changes: 14 additions & 0 deletions src/System Application/App/Agent/Setup/Agent.Codeunit.al
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ codeunit 4321 Agent
/// Archives the agent. Archiving removes the agent from active use and cannot be undone. The agent must be inactive (deactivated) before it can be archived.
/// </summary>
/// <param name="AgentUserSecurityID">The user security ID of the agent.</param>
/// <remarks>An error is raised if the agent's type does not support archiving. Call IsArchivingSupported to check before archiving.</remarks>
procedure Archive(AgentUserSecurityID: Guid)
var
AgentImpl: Codeunit "Agent Impl.";
Expand All @@ -69,6 +70,19 @@ codeunit 4321 Agent
AgentImpl.Archive(AgentUserSecurityID);
end;

/// <summary>
/// Checks if the agent type supports archiving.
/// </summary>
/// <param name="AgentUserSecurityID">The user security ID of the agent.</param>
/// <returns>True if the agent type supports archiving; otherwise false.</returns>
procedure IsArchivingSupported(AgentUserSecurityID: Guid): Boolean
var
AgentImpl: Codeunit "Agent Impl.";
begin
FeatureAccessManagement.AgentManagementAllowed(true);
exit(AgentImpl.IsArchivingSupported(AgentUserSecurityID));
end;

/// <summary>
/// Checks if the agent is archived.
/// </summary>
Expand Down
6 changes: 5 additions & 1 deletion src/System Application/App/Agent/Setup/AgentCard.Page.al
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ page 4315 "Agent Card"
Agent: Codeunit Agent;
ArchiveConfirmation: Page "Agent Archive Confirmation";
begin
if not Agent.IsArchivingSupported(Rec."User Security ID") then
Error(ArchivingNotSupportedErr, Rec."Agent Metadata Provider");

if Rec.State <> Rec.State::Disabled then
Error(DeactivateBeforeArchivingErr);

Expand Down Expand Up @@ -324,6 +327,7 @@ page 4315 "Agent Card"
YouCannotEnableAgentWithoutUsingConfigurationPageErr: Label 'You can''t activate the agent from this page. Use the action to configure and activate the agent.';
YouDoNotHavePermissionToModifyThisAgentErr: Label 'You do not have permission to modify this agent. Contact your system administrator to update your permissions or to mark you as one of the administrators for the agent.';
DeactivateBeforeArchivingErr: Label 'Deactivate the agent before archiving it.';
ArchivingNotSupportedErr: Label 'Archiving agents of type ''%1'' is not supported.', Comment = '%1 = the type of the agent.';
Comment thread
krupybalu marked this conversation as resolved.
AgentArchivedMsg: Label 'The agent has been archived.';
AgentArchivedNotificationMsg: Label 'This agent is archived and can no longer be modified. Its tasks and logs remain available for auditing.';
}
}
25 changes: 25 additions & 0 deletions src/System Application/App/Agent/Setup/AgentImpl.Codeunit.al
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,37 @@ codeunit 4301 "Agent Impl."
if Agent.Substate = Agent.Substate::Archived then
exit; // Archiving is terminal; idempotent no-op avoids the platform "archived agent cannot be modified" error on re-archive.

if not IsArchivingSupported(Agent) then
Comment thread
krupybalu marked this conversation as resolved.
Error(ArchivingNotSupportedErr, Agent."Agent Metadata Provider");

if Agent.State <> Agent.State::Disabled then
Error(DeactivateBeforeArchivingErr);

Agent.Substate := Agent.Substate::Archived;
Agent.Modify(true);
end;

procedure IsArchivingSupported(AgentUserSecurityID: Guid): Boolean
var
Agent: Record Agent;
begin
GetAgent(Agent, AgentUserSecurityID);

exit(IsArchivingSupported(Agent));
end;

local procedure IsArchivingSupported(Agent: Record Agent): Boolean
Comment thread
krupybalu marked this conversation as resolved.
var
AgentArchiving: Interface IAgentArchiving;
begin
if IsNullGuid(Agent."User Security ID") then
exit(false);

AgentArchiving := Agent."Agent Metadata Provider";
Comment thread
krupybalu marked this conversation as resolved.

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.

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

AgentImpl.IsArchivingSupported(Agent) (local overload) does AgentArchiving := Agent."Agent Metadata Provider"; exit(AgentArchiving.IsArchivingSupported());, which requires every enum value of "Agent Metadata Provider" to implement IAgentArchiving. Confirmed by inspecting the current enum extensions: the default "SDK Mock Agent" value (MockAgentMetaProv.EnumExt.al), "SO Agent" (SOAMetadataProvider.EnumExt.al) and "Custom Agent" (CustomAgentMetadataProvider.EnumExt.al) do NOT declare an IAgentArchiving implementation. Casting such a value to the IAgentArchiving interface variable raises an unhandled AL runtime error ("enum value does not implement interface") instead of returning a Boolean. This directly contradicts the new test assertion added in the same PR, Assert.IsTrue(Agent.IsArchivingSupported(AgentId), 'Archiving should be supported for a type that does not opt out') in AgentTest.Codeunit.al (ArchiveDisabledAgentSetsArchived, ~line 997), which calls IsArchivingSupported on an agent created via GetOrCreateDefaultAgent — i.e. an "SDK Mock Agent" — and expects true, but the call will error instead. The same crash risk applies in production to the AgentCard/AgentList archive actions for SO Agent and Custom Agent users. Add an explicit guard (e.g. check whether the interface is implemented, or default to a Boolean via a safe pattern) so agent types that have not opted into IAgentArchiving return a sane default instead of throwing.

Agent judgement — not directly backed by a BCQuality knowledge article.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4


exit(AgentArchiving.IsArchivingSupported());
end;

procedure IsArchived(AgentUserSecurityID: Guid): Boolean
var
Agent: Record Agent;
Expand Down Expand Up @@ -644,6 +668,7 @@ codeunit 4301 "Agent Impl."
AgentDoesNotExistErr: Label 'Agent does not exist.';
AgentArchivedCannotBeModifiedErr: Label 'The agent is archived and cannot be modified.';
DeactivateBeforeArchivingErr: Label 'Deactivate the agent before archiving it.';
ArchivingNotSupportedErr: Label 'Archiving agents of type ''%1'' is not supported.', Comment = '%1 = the type of the agent.';
AutoLbl: Label 'Auto';
NoActiveAgentsErr: Label 'There are no active agents setup on the system.';
NoAgentsAvailableNotificationLbl: Label 'Business Central agents are currently not available in your country.';
Expand Down
6 changes: 5 additions & 1 deletion src/System Application/App/Agent/Setup/AgentList.Page.al
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ page 4316 "Agent List"
if Rec.IsEmpty() then
Error(NoAgentSetupErr);

if not Agent.IsArchivingSupported(Rec."User Security ID") then
Error(ArchivingNotSupportedErr, Rec."Agent Metadata Provider");

if Agent.IsActive(Rec."User Security ID") then
Error(DeactivateBeforeArchivingErr);

Expand Down Expand Up @@ -302,4 +305,5 @@ page 4316 "Agent List"
AgentIsArchived: Boolean;
NoAgentSetupErr: Label 'No agents have been setup. You must set up an agent first.';
DeactivateBeforeArchivingErr: Label 'Deactivate the agent before archiving it.';
}
ArchivingNotSupportedErr: Label 'Archiving agents of type ''%1'' is not supported.', Comment = '%1 = the type of the agent.';
Comment thread
krupybalu marked this conversation as resolved.
}
33 changes: 33 additions & 0 deletions src/System Application/Test/Agent/src/SDK/AgentTest.Codeunit.al
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ codeunit 133961 "Agent Test"
LibraryVariableStorage: Codeunit "Library - Variable Storage";
DeactivateBeforeArchivingErr: Label 'Deactivate the agent before archiving it.', Locked = true;
AgentArchivedCannotBeModifiedErr: Label 'The agent is archived and cannot be modified.', Locked = true;
ArchivingNotSupportedErr: Label 'is not supported', Locked = true;

local procedure Initialize()
begin
Expand Down Expand Up @@ -993,6 +994,7 @@ codeunit 133961 "Agent Test"

Agent.Deactivate(AgentId);
Assert.IsFalse(Agent.IsArchived(AgentId), 'Agent should not be archived initially');
Assert.IsTrue(Agent.IsArchivingSupported(AgentId), 'Archiving should be supported for a type that does not opt out');

// [WHEN] Archiving the agent
Agent.Archive(AgentId);
Expand All @@ -1005,6 +1007,37 @@ codeunit 133961 "Agent Test"
Assert.AreEqual(AgentRecord.Substate::Archived, AgentRecord.Substate, 'Agent substate should be Archived');
end;

[Test]
procedure ArchiveAgentOfUnsupportedTypeErrors()
var
AgentRecord: Record Agent;
TempAgentAccessControl: Record "Agent Access Control" temporary;
Any: Codeunit Any;
AgentId: Guid;
AgentUserName: Code[50];
begin
Initialize();

// [SCENARIO] Archiving an agent whose type opts out of archiving is rejected

// [GIVEN] An agent of a type that reports archiving as unsupported
AgentUserName := CopyStr(Any.AlphanumericText(MaxStrLen(AgentRecord."User Name")), 1, MaxStrLen(AgentRecord."User Name"));
AgentId := Agent.Create(
"Agent Metadata Provider"::"SDK Mock Agent No Archiving",
AgentUserName,
CopyStr(Any.AlphanumericText(80), 1, 80),
TempAgentAccessControl);

// [THEN] Archiving is reported as unsupported for that agent
Assert.IsFalse(Agent.IsArchivingSupported(AgentId), 'Archiving should not be supported for this agent type');

// [WHEN] Archiving the agent
// [THEN] An error is raised and the agent is not archived
asserterror Agent.Archive(AgentId);
Assert.ExpectedError(ArchivingNotSupportedErr);

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.

$\textbf{🟡\ Medium\ Severity\ —\ Testing}$

This negative test follows asserterror with Assert.ExpectedError, but it only matches the generic substring is not supported. That is broad enough that an unrelated failure with the same wording could still make the test pass without proving the unsupported-archiving guard fired. Assert a more specific invariant fragment of the message (and the error code if known).

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4

Assert.IsFalse(Agent.IsArchived(AgentId), 'Agent should not be archived');
end;

[Test]
procedure ArchiveActiveAgentErrors()
var
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ codeunit 133954 "Library Mock Agent"
AgentRecord: Record Agent;
MockAgentSetup: Record "Mock Agent Setup";
begin
AgentRecord.SetRange("Agent Metadata Provider", AgentRecord."Agent Metadata Provider"::"SDK Mock Agent");
AgentRecord.SetFilter("Agent Metadata Provider", '%1|%2', AgentRecord."Agent Metadata Provider"::"SDK Mock Agent", AgentRecord."Agent Metadata Provider"::"SDK Mock Agent No Archiving");
if AgentRecord.FindSet() then
repeat
if MockAgentSetup.Get(AgentRecord."User Security ID") then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ using System.AI;
using System.Reflection;
using System.Security.AccessControl;

codeunit 133952 "Mock Agent Meta. Prov." implements IAgentMetadata, IAgentFactory
codeunit 133952 "Mock Agent Meta. Prov." implements IAgentMetadata, IAgentFactory, IAgentArchiving
{
InherentEntitlements = X;
InherentPermissions = X;
Expand All @@ -21,6 +21,11 @@ codeunit 133952 "Mock Agent Meta. Prov." implements IAgentMetadata, IAgentFactor
MockAgentSetup: Codeunit "Mock Agent Setup";
MockAgentInitialLbl: Label 'MA', MaxLength = 4;

procedure IsArchivingSupported(): Boolean
begin
exit(false);
end;

procedure GetDefaultInitials(): Text[4]
begin
exit(MockAgentInitialLbl);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,10 @@ enumextension 133952 "Mock Agent Meta. Prov." extends "Agent Metadata Provider"
Caption = 'SDK Mock Agent';
Implementation = IAgentFactory = "Mock Agent Meta. Prov.", IAgentMetadata = "Mock Agent Meta. Prov.";
}

value(133955; "SDK Mock Agent No Archiving")
{
Caption = 'SDK Mock Agent No Archiving';
Implementation = IAgentFactory = "Mock Agent Meta. Prov.", IAgentMetadata = "Mock Agent Meta. Prov.", IAgentArchiving = "Mock Agent Meta. Prov.";
}
}
Loading