Ability to archive agents#8967
Conversation
Slice 620767. Foundational app-side archiving in the System Application Agent module: - Public Agent.Archive(Guid) + Agent.IsArchived(Guid) (gated via FeatureAccessManagement). - AgentImpl.Archive: idempotent no-op if already archived; requires State=Disabled (else DeactivateBeforeArchivingErr); sets Substate=Archived; Modify(true) so the platform None->Archived callback fires. - New page 4337 'Agent Archive Confirmation': Azure-style type-the-exact-name (case-sensitive) confirmation warning the action cannot be undone. - Agent List + Card: Archive action (enabled only when Inactive and not archived), Substate column, Activate/Setup disabled for archived agents. - 4 SDK tests (execution on gate; no AL server locally). Compiles against local platform symbols (IAgentArchiving + Substate): BUILD OK. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Slice 620767 (b2). Archived agents (Substate=Archived) are excluded from the default Agent List (OnOpenPage SetRange Substate=None) and shown in a new read-only page 4339 'Archived Agents' (filter Substate=Archived), reachable via a navigation action. App-computed filtering on Substate, consistent with b1. 3 SDK tests added (execution on gate). Agent app: BUILD OK. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Slice 620767. Agent Task List (page 4300) excludes tasks of archived agents by default via the platform 'Agent Substate' flowfield (SetRange Agent Substate=None), with a Show all agents / Show active agents toggle (mirrors the Agent List company toggle) and a read-only Agent Substate column visible when showing all. Consumption overview is intentionally left unfiltered (different table). 2 SDK tests added (execution on gate). Agent app: BUILD OK. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Could not find a linked ADO work item. Please link one by using the pattern 'AB#' followed by the relevant work item number. You may use the 'Fixes' keyword to automatically resolve the work item when the pull request is merged. E.g. 'Fixes AB#1234' |
…ve an agent from active use while retaining it for audit. Archived agents are disabled, hidden from the Agent List/Role Center, frozen, and cannot be reconfigured. - AgentImpl: add Archive/IsArchived, plus an EnsureNotArchived guard on the satellite writes the platform freeze cannot see (user settings, permission sets, opening the setup page). Direct Agent-record changes are gated by the platform (Agent virtual data provider), so they need no guard here. - AgentArchiveConfirmation: require typing the agent name to confirm archiving. - AgentCard, AgentUserSettings, ViewAgentPermissions: disable configuration entry points for archived agents, and block persisting user settings on close (OnQueryClosePage), which uses a different write path than AgentImpl. - Tests: archiving + confirmation flow, facade immutability, disabled card actions, rejected profile/localization/permission-set edits on an archived agent, and blocked task creation for an archived agent.
… Substate filter (AB#620767)
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Copilot PR ReviewIteration 8 · Outcome: completed Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf Sub-skills skipped
Orchestrator pre-filter (2 file(s) excluded)
Findings produced by the AL review agent v1.7.3. Reply 👎 on any inline comment to flag false positives. |
Archiving an agent is documented as terminal ('removes the agent from active use and cannot be undone', see Agent.Archive doc comment), and the new EnsureNotArchived guard was added to UpdateAgentUserSettings, AssignPermissionSets, and OpenSetupPageId to enforce that.However, Agent.Activate (Agent.Codeunit.al line 40) / AgentImpl.Activate -> ChangeAgentState (AgentImpl.Codeunit.al line 49 and ~405) never checks Substate before setting State := Enabled. Any caller of the public Agent.Activate SDK procedure (extensions, automation, or a future UI action) can flip an archived agent's State back to Enabled while Substate remains Archived, and IsActive() will then report the agent as active again -- silently undoing the archive despite the stated 'cannot be undone' contract. The UI-level mitigations (StateEditable/ArchiveActionEnabled bindings in AgentCard.Page.al lines 147/258 and AgentList.Page.al lines 93/245) only prevent this from the Card/List pages; they do not close the gap at the codeunit API level. Recommendation:
Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why |
thloke
left a comment
thloke
left a comment
There was a problem hiding this comment.
In the platform changes, task modifications for archived agents will throw. I don't see an equivalent change here to disable any actions on tasks for archived agents.
JesperSchulz
added
Other
This PR introduces an "archived agents cannot be modified" invariant, enforced via a new local EnsureNotArchived guard in AgentImpl.Codeunit.al, and applies it to UpdateAgentUserSettings and AssignPermissionSets.However, the pre-existing public procedure UpdateAgentAccessControl(AgentUserSecurityID, TempAgentAccessControl) -- called directly from ViewAgentAccessControl.Page.al to add/remove/modify per-user "Can Configure Agent" access records -- does not call EnsureNotArchived and is not routed through AssignPermissionSets. An archived agent's access-control records can therefore still be inserted, updated, or deleted through that page after archiving, undermining the very guarantee this PR adds elsewhere (setup, localization, and permission-set assignment are all frozen, but this access-control update path is not). Recommendation:
Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why |
…#620767) Adds two tests requested in PR review (nikola): - ReactivateArchivedAgentErrors: an archived agent cannot be reactivated; the platform rejects Activate with "An archived agent cannot be modified." and the agent stays archived and inactive. - AddingMessageToArchivedAgentTaskIsBlocked: adding a message to a task whose owning agent is archived must be rejected. This test currently fails by design - the platform message-insert path has no archived-agent guard yet - and documents the intended behavior until that guard is added. Also fixes AA0021 variable-ordering (Codeunit before TestPage) in ArchivedAgentUserSettingsActionDisabledOnCard and ArchivedAgentConfigureActionEnabledOnCard so the Agent Test module compiles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a03f6f97-b656-48bd-9d27-ef87ddbd7d7b
Archive was enabled based only on agent state (Disabled and not archived), omitting the per-agent 'Can Curr. User Configure Agent' permission that the sibling Configure and Agent User Settings actions enforce. A user with per-agent access limited to a subset could archive any visible agent. Add the same permission check to ArchiveActionEnabled on both the Agent List and Agent Card pages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a03f6f97-b656-48bd-9d27-ef87ddbd7d7b
…pattern (AB#620767) Drive the AgentArchiveConfirmationModalHandler through Library - Variable Storage (Enqueue/Dequeue) instead of codeunit globals, add AssertEmpty() to each archive-confirmation test to prove the dialog fired exactly once, and clear the queue in Initialize() so no value leaks across tests. Follows the UI-handler testing guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a03f6f97-b656-48bd-9d27-ef87ddbd7d7b
…lied (AB#620767) UpdateAgentAccessControlOnArchivedAgentSucceeds previously asserted only that the access-control entry count was 1, which already holds from Agent.Create, so a no-op update would still pass. Stage a second user (not in the create baseline) in the buffer and assert it is present after the update, proving access control is genuinely administrable on an archived agent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a03f6f97-b656-48bd-9d27-ef87ddbd7d7b
ArchivedAgentUserSettingsActionDisabledOnCard and ArchivedAgentConfigureActionEnabledOnCard open the Agent Card for an archived agent, which fires OnAfterGetCurrRecord -> SendArchivedNotificationIfNeeded -> ArchivedNotification.Send(). In a headless test run an uncaptured Notification.Send() with no registered handler raises an unhandled-UI failure before the assertions run. Declare [HandlerFunctions] on both tests and add a minimal [SendNotificationHandler] that captures the notification. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a03f6f97-b656-48bd-9d27-ef87ddbd7d7b
| SendArchivedNotificationIfNeeded(); | ||
| end; | ||
|
|
||
| local procedure SendArchivedNotificationIfNeeded() |
There was a problem hiding this comment.
SendArchivedNotificationIfNeeded on the Agent Card page uses a single page-level Boolean (ArchivedNotificationShown) to decide whether to (re-)send the 'agent is archived' notification. The flag is only reset to false when the current record is not archived; it is never reset when the user navigates directly from one archived agent to a different archived agent (e.g. via next/previous record or a filtered list). In that case the flag is already true from the first archived agent, so the notification is silently skipped for the second archived agent even though it is a different record the user has never seen the notification for. Fix by keying the 'already shown' state to the current record instead of a plain flag, e.g. by storing the last-notified agent's 'User Security ID' and comparing it against Rec."User Security ID" instead of a plain Boolean.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.12.4
| AgentTaskMessageBuilder.Initialize(MessageFrom, MessageText); | ||
| AgentTaskMessageBuilder.SetAgentTask(Task.Id); | ||
| asserterror AgentTaskMessageBuilder.Create(); | ||
| Assert.ExpectedError('is not allowed because the associated agent is archived'); |
There was a problem hiding this comment.
AddingMessageToArchivedAgentTaskIsBlocked (added in this PR) does asserterror AgentTaskMessageBuilder.Create(); and then Assert.ExpectedError('is not allowed because the associated agent is archived');, but no code path reachable from that call raises this text: AgentTaskMsgBuilderImpl.Create -> AgentTaskImpl.AddMessage contains no archived-agent guard anywhere in this diff or the pre-existing codebase, and the string does not appear anywhere else in the repository's AL source. As written, this test will fail (or is dead code testing behavior that was never implemented) since nothing blocks adding a message to an archived agent's task; every other archived-agent guard added in this PR (EnsureNotArchived in AgentImpl.Codeunit.al) protects only UpdateAgentUserSettings and AssignPermissionSets, not message creation on Agent Task. If blocking messages on archived-agent tasks is intended, the corresponding guard/error needs to be added to AgentTaskImpl.AddMessage (or AgentTaskMsgBuilderImpl.Create); if it is not intended, this test and its scenario comment are incorrect and should be removed or corrected. Given the concrete CI-breaking impact, this should be resolved before merge even though it is emitted here at capped agent-finding severity.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.12.4
6 participants
What & why
Linked work
Fixes #
How I validated this
What I tested and the outcome (required — be specific: scenarios, commands, screenshots for UI changes)
Risk & compatibility