Repository navigation
feat(core): edit saved permissions - #56659
rekram1-node wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Looks sound. I ran the new server test, which goes through the real request handler and the real "always" reply flow (PATCH returns the updated entry, the list shows the edit, a duplicate gets 409, an unknown ID gets 404), and the core permission tests (106 pass). Type checks are clean in core, protocol, server and client.
Public API: adds PATCH /api/permission/saved/:id (permission.saved.update, payload { action?, resource? } → { data: PermissionSaved.Info }, errors 404 PermissionSavedNotFoundError / 409 ConflictError). Also adds the PermissionSavedNotFoundError error schema, saved.update in the generated effect and promise clients, and PermissionSaved.update, UpdateInput, NotFoundError and ConflictError in core. Additive only, so it is backwards compatible.
One small concern, inline: the duplicate check isn't atomic with the write.
Related: #50600
| .get() | ||
| .pipe(Effect.orDie) | ||
| if (duplicate) return yield* new ConflictError({ id: duplicate.id, action, resource }) | ||
| const updated = yield* db |
There was a problem hiding this comment.
The duplicate check and the update aren't atomic. If two edits run at once, or an "always" reply adds the same rule in between, this update hits permission_project_action_resource_idx, and Effect.orDie turns that into a 500 instead of the 409. Wrapping the select and the update in a transaction, or mapping the unique-constraint failure to ConflictError, would keep the response consistent.
Problem
Saved permissions ("Always allow" answers) can be listed and removed through the API, but not changed. To narrow a broad approval, such as a saved
shell: *, a client has to delete it, and there is no way to recreate a narrower one. This blocks a UI for reviewing saved approvals (#50600).Change
Adds
PATCH /api/permission/saved/:id(permission.saved.update), which changes a saved permission'sactionand/orresource. The project can't change, and the rule stays anallow./api/permission/saved GET permission.saved.list + PATCH permission.saved.update { action?, resource? } → { data: PermissionSaved.Info } DELETE permission.saved.removeThe edit applies to the next permission check, like any saved permission. Requests already waiting are not re-checked, the same as
remove.packages/core/src/permission/saved.ts:update, plusNotFoundErrorandConflictErrorpackages/protocol: the endpoint and a newPermissionSavedNotFoundError(the existingPermissionNotFoundErroris for pending requests)packages/server/src/handlers/permission.ts: maps the core errors to 404 and 409packages/client: regeneratedOut of scope
addandremovedon't publish one either.ask(today a savedallowwins; see permissions: V2 “Always allow” silently overrides configured “ask” across future project sessions #48456).Verification
packages/core/test/permission.test.ts: editing narrows what a saved permission allows (the edited resource is allowed, the old pattern prompts again), and conflict and not-found errors.bun run script/test.ts test/permission.test.ts --rerun-each 5: 530 pass.packages/server/test/permission-saved.test.ts: creates two saved permissions through the real request and "always" reply flow, then checks PATCH returns the updated entry, the list reflects it, a duplicate returns 409, and an unknown ID returns 404.bun test test/permission-saved.test.ts --rerun-each 10: 10 pass.bun run check: 34 of 34 tasks pass.Added by the opencode bot (AI-generated).
Related: #50600
Public API: adds PATCH /api/permission/saved/:id (permission.saved.update) and client saved.update (backwards compatible)