Feat/access errors by SharonStrats · Pull Request #893 · SolidOS/solid-ui

SharonStrats · 2026-10-14T02:14:32Z

This PR does a major refactor of the code so that all the code is not in one file and many things were simplified.

It also adds error handling using the debug for console logs, displaying errors to the user, handling input errors up front. I'll explain a few details below as well I have included an example of how it will look to the user.

Behavior notes:
Adding new assures valid authorization targets before Save

When adding a new authorization the target must resolve to a supported access-control subject: agent/WebID, agent group, agent class, or origin, where supported. If the target cannot be verified, it is rejected and is not added to the pending list. If that happens in the UI it will say Could not verify this access target
If target resolution throws for adding input—for example, because classification or related lookup fails—the exception is caught. The error message will show and if no message this Could not verify this access target
When an error is received here the user can retype and the error is cleared.

All save errors appear in the dialog below

Prompt: Validate the input for each subjectType for agent and agentGroup make sure you can load the doc and see that it has a valid agent, agentGroup, agentClass type, validate origin based on format we have the check in solidlogic already add hostname. Co-authored-by: GPT-5.6 Luna <[email protected]>

Prompt: Create shared ACL view models instead of reconstructing them in multiple methods. then rerun unit and e2e tests. Co-authored-by: GPT-5.6 Luna <[email protected]>

…errors Prompt: When there are errors the results that were successful need to be shown to the user in the open dialog, but they still should not be able to retry unsuccessful updates. Co-authored-by: GPT-5.6 Terra <[email protected]>

Copilot

🟡 Changes recommended

Failed saves can leave misleading role state, stale validation errors, and duplicate close events.

4 open findings

Prevent duplicate close event dispatch · New
Reconcile roles after all change failures · New
Guard stale async validation errors · New
Fix contradictory revoke failure message · New

What changed in this PR

Adds granular access-control error handling while decomposing the modal into focused components.

Changes:

Reports individual save and validation failures.
Extracts access-target, grant-draft, and save logic.
Adds tests for partial failures and target resolution.

File	Description
`test/components/access-control-modal/buildAccessChanges.test.ts`	Tests change construction.
`test/components/access-control-modal/accessTargetResolver.test.ts`	Tests target resolution.
`test/components/access-control-modal/accessGrantDrafts.test.ts`	Tests grant drafts.
`test/components/access-control-modal.test.ts`	Expands modal integration tests.
`src/components/access-control-modal/types.ts`	Defines events, drafts, and errors.
`src/components/access-control-modal/helpers.ts`	Adds origin normalization.
`src/components/access-control-modal/buildAccessChanges.ts`	Builds queued ACL changes.
`src/components/access-control-modal/applyAccessChanges.ts`	Applies changes with per-item errors.
`src/components/access-control-modal/accessTargetResolver.ts`	Resolves and verifies targets.
`src/components/access-control-modal/accessGrantDrafts.ts`	Manages grant draft models.
`src/components/access-control-modal/AccessControlSharedAccess.ts`	Extracts shared-access UI.
`src/components/access-control-modal/AccessControlSharedAccess.styles.css`	Styles shared-access UI.
`src/components/access-control-modal/AccessControlModal.ts`	Coordinates saving and errors.
`src/components/access-control-modal/AccessControlModal.styles.css`	Retains modal-level styles.
`src/components/access-control-modal/AccessControlGeneralAccess.ts`	Extracts general-access UI.
`src/components/access-control-modal/AccessControlGeneralAccess.styles.css`	Styles general-access UI.
`src/components/access-control-modal/AccessControlErrors.ts`	Renders save failures.
`src/components/access-control-modal/AccessControlErrors.styles.css`	Styles failure details.
`src/components/access-control-modal/AccessControlAddAccess.ts`	Extracts grant-entry UI.
`src/components/access-control-modal/AccessControlAddAccess.styles.css`	Styles grant-entry UI.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…[email protected]) (latest: [email protected])

Prompt: Pasted the errors from the ci into the chat - can you see why these errors would happen in ci but not locally and fix. Co-authored-by: GPT-5.6 Luna <[email protected]>

Copilot

🟡 Changes recommended

Validation races, discarded lookup messages, and non-unique draft identifiers can produce incorrect or missing UI state.

5 open findings

4 resolved since last review

🧠 Review effort: Balanced

timea-solid

I looked at the code in general and it looks ok. A lot of great work!

NoelDeMartin

I haven't look at all the code, but I noticed no actual new components are exported so that looks good to me. Anything that involves simplyfing and refactoring code into smaller files is usually a good thing :).

TallTed · 2026-10-14T21:53:23Z

+        title="Some access changes could not be saved"
+        message=${this.successfulChangesRefreshed
+          ? 'Any successful changes are shown above. The list below shows only the changes that failed. Close this dialog and reopen it if you want to try again.'
+          : 'Some changes may have been saved, but we could not refresh the Shared with list. The list below shows only the changes that failed. Close this dialog and reopen it if you want to try again.'}


the Shared with list is a terrible name or title reference. Maybe it could be changed to the "Shared with" list or something along those lines? Style changes can also help deliver the clarity I'm hoping for.

Do you have other suggestions? What do you mean style changes? Do you have style change suggestions for the errors I have put the image in the description? I am open to ideas. I probably shouldn't change the "Shared with " header though bc that was part of the design but the message and the way I have displayed the errors can be changed.

@TallTed I'm going to need to push this through because we are on a timeline, but I would love to hear your feedback and have a discussion. There is still time to adjust before releasing it. We have another week probably a bit longer. You'll be able to see it on https://pivot-test.solidproject.org:3000 by tomorrow if not before depending on your timezone. Any feedback please create a ticket on solid-ui.

…[email protected]) (latest: [email protected])

SharonStrats and others added 7 commits October 13, 2026 18:04

add error handling

19ee222

Clean up

f90f2c0

Prompt: Create shared ACL view models instead of reconstructing them in multiple methods. then rerun unit and e2e tests. Co-authored-by: GPT-5.6 Luna <[email protected]>

code cleanup to prepare for refactor

ff093f7

basic refactor splitting the access modal

20aa9bf

more refactoring and cleanup

3ace3b4

SharonStrats requested a balanced review from Copilot October 14, 2026 02:14

SharonStrats self-assigned this Oct 14, 2026

SharonStrats added the enhancement label Oct 14, 2026

SharonStrats added this to SolidOS NLNet UI Oct 14, 2026

Copilot started reviewing on behalf of SharonStrats October 14, 2026 02:15

View session

Copilot AI reviewed Oct 14, 2026

View reviewed changes

SharonStrats added 3 commits October 14, 2026 13:51

adjust styles

59ce76f

address PR feedback

2b075d9

Merge branch 'staging' into feat/access-errors

ccf79f6

SharonStrats force-pushed the feat/access-errors branch from b549cae to ccf79f6 Compare October 14, 2026 03:07

SharonStrats marked this pull request as ready for review October 14, 2026 03:07

SharonStrats requested review from NoelDeMartin and timea-solid October 14, 2026 03:07

SharonStrats moved this to In review in SolidOS NLNet UI Oct 14, 2026

chore: update solidos dependencies (dev: [email protected] pane-reg…

7a145ce

…[email protected]) (latest: [email protected])

SharonStrats linked an issue Oct 14, 2026 that may be closed by this pull request

Sharing pane: Add error handling #894

Open

Fixed test issues in CI - they were not local though for some reason

90bafaf

Prompt: Pasted the errors from the ci into the chat - can you see why these errors would happen in ci but not locally and fix. Co-authored-by: GPT-5.6 Luna <[email protected]>

SharonStrats force-pushed the feat/access-errors branch from c41b3f1 to 90bafaf Compare October 14, 2026 03:31

SharonStrats requested a balanced review from Copilot October 14, 2026 03:36

Copilot started reviewing on behalf of SharonStrats October 14, 2026 03:36

View session

Copilot AI reviewed Oct 14, 2026

View reviewed changes

timea-solid approved these changes Oct 14, 2026

View reviewed changes

NoelDeMartin approved these changes Oct 14, 2026

View reviewed changes

TallTed reviewed Oct 14, 2026

View reviewed changes

address PR feedback

664919c

SharonStrats force-pushed the feat/access-errors branch from 7722a33 to 664919c Compare October 15, 2026 23:10

chore: update solidos dependencies (dev: [email protected] pane-reg…

5df5682

…[email protected]) (latest: [email protected])

SharonStrats merged commit 7411e01 into staging Oct 15, 2026
9 checks passed

github-project-automation Bot moved this from In review to Done in SolidOS NLNet UI Oct 15, 2026

SharonStrats deleted the feat/access-errors branch October 15, 2026 23:15

Navigation Menu

Repository navigation

Feat/access errors - #893

Feat/access errors#893
SharonStrats merged 14 commits into
stagingfrom
feat/access-errors

SharonStrats commented Oct 14, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timea-solid left a comment

Uh oh!

NoelDeMartin left a comment

Uh oh!

TallTed Oct 14, 2026

Uh oh!

SharonStrats Oct 14, 2026

Uh oh!

SharonStrats Oct 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

SharonStrats commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timea-solid left a comment

Choose a reason for hiding this comment

Uh oh!

NoelDeMartin left a comment

Choose a reason for hiding this comment

Uh oh!

TallTed Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

SharonStrats Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

SharonStrats Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SharonStrats commented Oct 14, 2026 •

edited

Loading