Add read-only package broker policy inspector - #5292
Add read-only package broker policy inspector#5292Benoît Cortier (CBenoit) wants to merge 9 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a Windows-only, read-only Devolutions Agent package-policy inspector integrated into Administrator settings.
Changes:
- Adds broker policy retrieval, validation, status mapping, and centralized client creation.
- Adds searchable, localized UI for policy metadata, rules, constraints, and canonical JSON.
- Adds coverage for broker responses, cancellation, stale refreshes, and search visibility.
The unpublished 0.4.0 dependencies remain a merge blocker.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Reject malformed required policy data using schema-aligned validation and cover missing, defaulted, nested, and wire-invalid responses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the settings shell as the single page-level heading for assistive technologies. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Match the repository formatter so the code-style CI check passes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Update the package broker inspector to the official 2026.9.3 API and Client packages and adopt the renamed source-generated serializers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The local validator permits two-value boolean match arrays despite the shared schema limiting them to one item.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 1
- Review effort level: Balanced
Reject multi-value boolean policy match arrays and cover the schema boundary with an inspector regression test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Schema-valid whitespace-only policy values are incorrectly hidden or represented as unset in the structured inspector.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/AgentPolicyInspectorViewModel.cs:295
- Schema-valid match and constraint strings only require a nonzero length, but this filter drops whitespace-only values. A policy containing
" "is therefore shown asAny/Noneeven though that value is an actual criterion or constraint. Preserve all validated entries when formatting the list.
This issue also appears on line 315 of the same file.
src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/AgentPolicyInspectorViewModel.cs:316
- This also conflates whitespace-only metadata with an absent value. The inspector explicitly accepts a single-space publisher as schema-valid, but
Valuerenders it as “Not set,” so the structured view no longer faithfully represents the active policy. Only treat null or empty strings as absent.
private static string Value(string? value, string fallback = "Not set") =>
string.IsNullOrWhiteSpace(value) ? CoreTools.Translate(fallback) : value;
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Keep schema-valid whitespace-only metadata, match, and constraint values visible and covered through accessibility and canonical JSON copy paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Structured 404 NotFound responses are incorrectly presented as unsupported Agent versions.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/UniGetUI.PackageEngine.AgentBroker/BrokerPolicyInspector.cs:292
- A structured
404 NotFoundis the contract response when a compatible Agent has no active policy, but this branch maps it toUnsupported; the UI then incorrectly tells users to update an Agent that already supports inspection. ReserveUnsupportedfor legacy 404s without a parsed broker error, and map structuredNotFoundtoPolicyUnavailable(updating the corresponding test expectation).
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Treat a parsed NotFound broker response as policy unavailable while preserving legacy empty 404 responses as unsupported Agents. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
URL validation accepts schema-invalid support URLs, and asynchronous results are not announced to assistive technology.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/AgentPolicyInspectorViewModel.cs:183
- The final success/failure status is populated asynchronously, but changing this
InfoBardoes not raise a live-region announcement. Screen-reader users therefore receive no notification that loading completed unless they navigate back to the status; announce the resulting title/message through the repository'sAccessibilityAnnouncementService(assertive for errors, polite otherwise).
- Files reviewed: 20/20 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The implementation spans policy-contract validation, broker transport, and UI behavior while its coordinated Agent dependency remains a draft.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Coordinated Agent compatibility and the security-sensitive policy-validation surface require final human verification.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Summary
BrokerClient.GetPolicyand sharedPolicyDocument/PolicySerializerwithout raw transport, duplicate DTOs, filesystem access, or mutationUseAgentBroker, cancellable, stale-safe, localized, searchable, accessible, and NativeAOT-safeInvalidResponseCoordinated Phase 1 work
Published dependency evidence
This branch consumes the official NuGet.org packages
Devolutions.Now.Policy.Apiand.Clientversion2026.9.3;.Modelversion2026.9.3is resolved transitively. A forced restore ofsrc/UniGetUI.Windows.slnxused a new empty package cache and onlyhttps://api.nuget.org/v3/index.json.Devolutions.Now.Policy.Model.2026.9.3.nupkg968001b5ac049c02e94ed3c8a60e57d82112480c010d82180659f0acb3add47dDevolutions.Now.Policy.Api.2026.9.3.nupkgebc532fb751a2e3d1c03ea7a685ac75f436d67126afb57cbdabd6245bf105163Devolutions.Now.Policy.Client.2026.9.3.nupkg59157e8f7f3558c75ce404110842c0cc48f290c12004627a6e24f70f1823319cCompatibility changes are limited to the official version pins, the published
PolicySerializer/BrokerSerializerAPI names, and strict-valid test fixture construction. Phase 1 remains read-only.Validation
net10.0andnet10.0-windows10.0.26100.0), 170 executions totalCopilot review dispositions
404 NotFoundbroker errors (PolicyUnavailable) from legacy empty/unparsed 404 responses (Unsupported)http://orhttps://support URL prefixes and added anhttp:fooinvalid-response regressionUnauthorizedAccessExceptionwrapped as broker-unavailable toAccessDeniedThis PR remains draft for coordinated stack finalization; the unpublished-package blocker is cleared. Phase 2 PR #5335 will restack after this base advances. The non-Windows runtime matrix remains a runner gap; defensive platform guards and Windows-only settings-search filtering are unit-tested.