fix(auth/command): team-scoped palette actions + team-access editor clarity - #496
Open
enyineer wants to merge 3 commits into
Open
fix(auth/command): team-scoped palette actions + team-access editor clarity#496enyineer wants to merge 3 commits into
enyineer wants to merge 3 commits into
Conversation
The palette filtered commands by GLOBAL access rules only, so a user whose team holds a create-capability grant (and no global *.manage rule) never saw "Create Incident" / "Create Maintenance" or their shortcuts - it hid the actions from exactly the people authorized to run them. Commands may now declare `manageCapability`, mirroring the gate routes/nav use. filterByAccessRules admits an item on the global rules OR a team grant on the declared type; the backend resolves that per request via hasAnyTypeGrant (includeCreator, so a would-be creator qualifies before owning an instance) and fails closed. Incident + maintenance commands declare their types. useGlobalShortcuts drops its userAccessRules re-check: the server-filtered list is authoritative, the check tested global rules only, and both call sites already defeated it by passing ["*"]. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hccsjzvqr6xfhGicNBjRgB
From user feedback on the "Who can change this" editor: - Relabel the "Manage" checkbox to "Can edit" and drop its gear icon. It sets the team's grant on THIS resource, but the wording + gear read as "manage the team", so people expected it to open the Teams UI. - Make the team name a link to /teams?team=<id>, which opens that team's members dialog. "Go to the team" now has its own affordance instead of being conflated with the grant checkbox. TeamsTab consumes the param once, then clears it. - Confirm before a user revokes their OWN team's only edit grant: afterwards they could neither change the resource nor restore the permission. Global teams.manage admins are exempt (they can always restore). The decision is extracted as the pure, unit-tested isSelfRevokingChange. - Explain the add-member field: it adds a NEW member from the directory (not a filter over current members), and only users who have signed in at least once exist there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hccsjzvqr6xfhGicNBjRgB
🦋 Changeset detectedLatest commit: 303fcd9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 80 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
❌ PR Checks Failed
❌ Security Audit Failures (dependency graph)How to fix: This gate scans the whole dependency graph (incl. devDependencies) and fails only on findings with an upgrade path (a fixed version exists); unfixed findings are warnings, not gated. Run @enyineer The above code quality issues were found in this PR. Please fix them before merging. |
The system overview listed a system's incidents/maintenances but gave no way to open one for it - you had to go to the incidents page and re-pick the system. Both system panels now offer "Report incident" / "Schedule maintenance", deep -linking to the editor with the system pre-selected via ?action=create&systemId=<id>. The config pages consume and clear both params. The action is gated on useProcedureAccess over the CREATE procedure's contract, so it shows for a global manager AND for someone who manages the system via a team (create.parent) - the bare global rule would have hidden it from exactly the team-scoped users it helps most. Editors gain `presetSystemIds`, and their unsaved-changes baseline accounts for it so opening a pre-scoped form and closing it does not nag to discard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hccsjzvqr6xfhGicNBjRgB
Contributor
❌ PR Checks Failed
❌ Security Audit Failures (dependency graph)How to fix: This gate scans the whole dependency graph (incl. devDependencies) and fails only on findings with an upgrade path (a fixed version exists); unfixed findings are warnings, not gated. Run @enyineer The above code quality issues were found in this PR. Please fix them before merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses 5 of the 6 feedback notes. (The "Manage X / Create X feel duplicated" note was dropped — both commands stay, per feedback that having both is good.)
1. Bug — Command Palette hid create-actions from team members
command-backendfiltered commands against the caller's global access rules only, so a user whose team holds a create-capability grant (but no globalincident.incident.manage) never saw "Create Incident" / "Create Maintenance" or their shortcuts. The palette hid the actions from exactly the people authorized to run them — the same "global-only gate excludes team-scoped users" class as.claude/rules/rlac.md.Commands may now declare
manageCapability(mirroring routes/nav).filterByAccessRulesadmits an item on the global rules OR a team grant on the declared type; the backend resolves that per request viahasAnyTypeGrant(withincludeCreator, so a would-be creator qualifies before owning an instance) and fails closed.Also removed
useGlobalShortcuts'userAccessRulesre-check: it tested global rules only and would have dropped team users' shortcuts — both call sites already defeated it by passing["*"], so it was dead, misleading code.2. Team-access editor clarity + safety
/teams?team=<id>, opening that team's members dialog. "Go to the team" gets its own affordance instead of being conflated with the checkbox. The Teams page consumes the param once, then clears it.teams.manageadmins are exempt (they can always restore). Extracted as the pure, unit-testedisSelfRevokingChange.Verification
typecheck✓ ·lint✓ · full unit suite 10,315 pass / 0 failfilter-access.test.ts(10 cases, pins the palette regression) andselfLockout.test.ts(8 cases)3. Create an incident / maintenance from the system overview
The system overview listed a system.s incidents and maintenances but offered no way to open one for it - you had to navigate away and re-pick the system by hand.
Both panels now carry an action ("Report incident" / "Schedule maintenance") that deep-links to the editor with the system already selected, via
?action=create&systemId=<id>. The pages consume and clear both params, so a refresh does not reopen the dialog.The action is gated on
useProcedureAccessover the CREATE procedure.s contract, so it appears for a global manager AND for someone who can manage that system through a team (thecreate.parentgate) - exactly who the backend accepts. Gating on the bare global rule would have hidden it from the team-scoped users it helps most (the same mistake as bug 1).Editors gain
presetSystemIds, and their unsaved-changes baseline accounts for the pre-selection - otherwise opening a pre-scoped form and closing it would falsely prompt to discard.Verification
typecheckOK,lintOK, full unit suite 10,315 pass / 0 failfilter-access.test.ts(10 cases, pins the palette regression) andselfLockout.test.ts(8 cases)Only the "Manage X / Create X feel duplicated" note is intentionally not addressed - per feedback, having both commands is good.